From 4d574982a173fa070170008ddd0e26c826cbcf23 Mon Sep 17 00:00:00 2001 From: Bretton <36870434+BrettM86@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:03:45 -0700 Subject: [PATCH] fix(comments): thread reply-pagination cursors and harden subtree merge per multi-model review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Section-2 second-opinion findings (6 reviewers, cursor bug unanimous): - store each node's repliesCursor and pass it on load-more, appending URI-deduplicated pages — previously the cursor was discarded and any comment with more than one page of direct replies refetched page 1 forever - merge subtree responses URI-keyed instead of last-write-wins so ancestor re-hydration preserves deeper branches loaded via nested load-more - deep replies skip the unsatisfiable top-level verify loop and retry the parent-subtree hydration instead; top-level retries use a quiet refresh (no full-list loading flicker); retry loop stops on dispose - vote state initializes only for newly fetched nodes (no optimistic revert) - loadMoreReplies contract: in-flight dedup returns the shared future, malformed URI throws, empty response clears hasMore, responses are anchor-validated and generation-guarded against refresh/sort races - FocusedThreadScreen: mounted guards after awaits, provider-authoritative anchor resolution with request sequencing, anchor-level load-more, retryable hydration-failure state, ValueKeys on stateful sibling cards - CommentView: tombstone invariant assert + isTombstoned getter; replaceDescendant preserves reference identity on miss - tests: focused-thread group unskipped (14 tests via shared test_helpers mocks), two-page cursor regression, deep-merge preservation, in-flight dedup, model tree-op units; stale smoke/router tests updated for the app-bar title and legal-gate providers Co-Authored-By: Claude Fable 5 --- lib/models/comment.dart | 62 +- lib/providers/comments_provider.dart | 433 ++++++++-- lib/screens/home/focused_thread_screen.dart | 112 ++- lib/screens/home/post_detail_screen.dart | 40 +- lib/widgets/comment_card.dart | 11 +- lib/widgets/comment_thread.dart | 61 +- test/models/comment_test.dart | 148 ++++ test/providers/comments_provider_test.dart | 518 +++++++++++- test/router/post_route_test.dart | 9 +- test/test_helpers/test_mocks.dart | 22 + test/test_helpers/test_mocks.mocks.dart | 846 +++++++++++++++++++ test/widget_test.dart | 42 +- test/widgets/comment_card_test.dart | 18 +- test/widgets/comment_thread_test.dart | 88 +- test/widgets/focused_thread_screen_test.dart | 625 +++++++++++--- test/widgets/post_detail_loader_test.dart | 103 ++- 16 files changed, 2838 insertions(+), 300 deletions(-) create mode 100644 test/test_helpers/test_mocks.dart create mode 100644 test/test_helpers/test_mocks.mocks.dart diff --git a/lib/models/comment.dart b/lib/models/comment.dart index 371a567..5948461 100644 --- a/lib/models/comment.dart +++ b/lib/models/comment.dart @@ -42,9 +42,10 @@ class CommentsResponse { class ThreadViewComment { ThreadViewComment({ required this.comment, - this.replies, + List? replies, this.hasMore = false, - }); + this.repliesCursor, + }) : replies = replies == null ? null : List.unmodifiable(replies); factory ThreadViewComment.fromJson(Map json) { return ThreadViewComment( @@ -64,28 +65,51 @@ class ThreadViewComment { } final CommentView comment; + + /// Direct replies to this comment. Unmodifiable (wrapped at construction) + /// so the tree stays immutable-by-convention. final List? replies; final bool hasMore; + /// Pagination cursor for this node's direct replies. + /// + /// Client-side state (never present in server JSON): set when a + /// load-more-replies subtree fetch returns a cursor, and passed back on + /// the next fetch so subsequent pages of direct replies can be appended + /// rather than refetching page 1 forever. Null when there is no next page. + final String? repliesCursor; + /// Creates a copy with the given fields replaced. /// /// Used when merging a freshly fetched subtree (load-more replies) into /// an existing comment tree without mutating the original nodes. + /// + /// [repliesCursor] uses a sentinel so callers can distinguish "not + /// provided" (keep current value) from "explicitly set to null" (clear + /// the cursor, e.g. when the last page has been fetched). ThreadViewComment copyWith({ CommentView? comment, List? replies, bool? hasMore, + Object? repliesCursor = _sentinel, }) { return ThreadViewComment( comment: comment ?? this.comment, replies: replies ?? this.replies, hasMore: hasMore ?? this.hasMore, + repliesCursor: + repliesCursor == _sentinel + ? this.repliesCursor + : repliesCursor as String?, ); } /// Returns this subtree with the node whose URI matches [replacement] - /// (this node or any descendant) replaced by [replacement]. Returns the - /// subtree unchanged when no node matches. + /// (this node or any descendant) replaced by [replacement]. + /// + /// Preserves reference identity on a miss: when no node matches, returns + /// `this` (and untouched sibling branches keep their identity), so callers + /// can detect a no-op merge via `identical(result, root)`. ThreadViewComment replaceDescendant(ThreadViewComment replacement) { if (comment.uri == replacement.comment.uri) { return replacement; @@ -94,12 +118,19 @@ class ThreadViewComment { if (currentReplies == null || currentReplies.isEmpty) { return this; } - return copyWith( - replies: - currentReplies - .map((reply) => reply.replaceDescendant(replacement)) - .toList(), - ); + var changed = false; + final mapped = []; + for (final reply in currentReplies) { + final result = reply.replaceDescendant(replacement); + if (!identical(result, reply)) { + changed = true; + } + mapped.add(result); + } + if (!changed) { + return this; + } + return copyWith(replies: mapped); } /// Depth-first search for a comment with the given [uri] in this subtree. @@ -156,6 +187,10 @@ class CommentView { }) : assert( !isDeleted || record == null, 'Deleted comments must have null record', + ), + assert( + author != null || isDeleted, + 'Non-deleted comments must have an author', ); factory CommentView.fromJson(Map json) { @@ -257,6 +292,13 @@ class CommentView { /// /// Returns null when [record] is null or when the comment has no facets. List? get contentFacets => record?.facets; + + /// Whether this comment should render as a tombstone (deleted placeholder). + /// + /// True when the comment is deleted or the backend withheld the author + /// (which it only does for deleted content). Prefer this over checking + /// [isDeleted] alone when deciding whether author info can be rendered. + bool get isTombstoned => isDeleted || author == null; } class CommentRef { diff --git a/lib/providers/comments_provider.dart b/lib/providers/comments_provider.dart index 93c0ea1..576a2d8 100644 --- a/lib/providers/comments_provider.dart +++ b/lib/providers/comments_provider.dart @@ -1,4 +1,4 @@ -import 'dart:async' show Timer, unawaited; +import 'dart:async' show Completer, Timer, unawaited; import 'package:characters/characters.dart'; import 'package:flutter/foundation.dart'; @@ -31,10 +31,13 @@ class CommentsProvider with ChangeNotifier { CovesApiService? apiService, VoteProvider? voteProvider, CommentService? commentService, + List? indexingRetryDelays, }) : _postUri = postUri, _postCid = postCid, _voteProvider = voteProvider, - _commentService = commentService { + _commentService = commentService, + _indexingRetryDelays = + indexingRetryDelays ?? _defaultIndexingRetryDelays { // Use injected service (for testing) or create new one (for production) // Pass token getter, refresh handler, and sign out handler to API service // for automatic fresh token retrieval and automatic token refresh on 401 @@ -54,10 +57,20 @@ class CommentsProvider with ChangeNotifier { /// Default staleness threshold for background refresh static const Duration stalenessThreshold = Duration(minutes: 5); + /// Default backoff schedule while waiting for the AppView to index a + /// newly created comment (~1-2s firehose lag). Injectable via the + /// constructor so tests don't need real delays. + static const List _defaultIndexingRetryDelays = [ + Duration(milliseconds: 400), + Duration(milliseconds: 800), + Duration(milliseconds: 1200), + ]; + final AuthProvider _authProvider; late final CovesApiService _apiService; final VoteProvider? _voteProvider; final CommentService? _commentService; + final List _indexingRetryDelays; // Post context - immutable per provider instance final String _postUri; @@ -67,15 +80,23 @@ class CommentsProvider with ChangeNotifier { List _comments = []; bool _isLoading = false; bool _isLoadingMore = false; + bool _isQuietLoading = false; String? _error; String? _cursor; bool _hasMore = true; + // Bumped whenever the whole tree is replaced (refresh/sort change/delete). + // In-flight subtree fetches capture it at start and discard their response + // if it changed, so a stale subtree is never merged into a newer tree. + int _treeGeneration = 0; + // Collapsed thread state - stores URIs of collapsed comments final Set _collapsedComments = {}; - // Comment URIs with an in-flight "load more replies" subtree fetch - final Set _loadingMoreReplies = {}; + // In-flight "load more replies" subtree fetches, keyed by comment URI. + // Duplicate calls for the same URI get the existing future back so every + // caller receives the real result instead of null. + final Map> _loadingMoreReplies = {}; // Scroll position state (replaces ScrollStateService for this post) double _scrollPosition = 0; @@ -117,7 +138,11 @@ class CommentsProvider with ChangeNotifier { String? get timeframe => _timeframe; ValueNotifier get currentTimeNotifier => _currentTimeNotifier; Set get collapsedComments => Set.unmodifiable(_collapsedComments); - Set get loadingMoreReplies => Set.unmodifiable(_loadingMoreReplies); + + /// Comment URIs with an in-flight "load more replies" subtree fetch + /// (for spinner state in the UI). + Set get loadingMoreReplies => + Set.unmodifiable(_loadingMoreReplies.keys); double get scrollPosition => _scrollPosition; DateTime? get lastRefreshTime => _lastRefreshTime; @@ -225,9 +250,12 @@ class CommentsProvider with ChangeNotifier { /// /// Parameters: /// - [refresh]: Whether to refresh from the beginning (true) or paginate (false) - Future loadComments({bool refresh = false}) async { + /// - [quiet]: When refreshing, don't flip [isLoading] (no full-list loading + /// flicker). Used for background retries while waiting for the AppView to + /// index a newly created comment. + Future loadComments({bool refresh = false, bool quiet = false}) async { // If already loading, schedule a refresh to happen after current load - if (_isLoading || _isLoadingMore) { + if (_isLoading || _isLoadingMore || _isQuietLoading) { if (refresh) { _pendingRefresh = true; if (kDebugMode) { @@ -241,7 +269,13 @@ class CommentsProvider with ChangeNotifier { try { if (refresh) { - _isLoading = true; + if (quiet) { + // Internal re-entrancy guard only - not exposed via isLoading, so + // the UI keeps showing the current tree while we refresh behind it. + _isQuietLoading = true; + } else { + _isLoading = true; + } _error = null; _pendingRefresh = false; // Clear any pending refresh } else { @@ -266,6 +300,9 @@ class CommentsProvider with ChangeNotifier { if (refresh) { _comments = response.comments; _lastRefreshTime = DateTime.now(); + // The whole tree was replaced - invalidate in-flight subtree fetches + // so they don't merge stale data into the new tree. + _treeGeneration++; } else { // Create new list instance to trigger rebuilds _comments = [..._comments, ...response.comments]; @@ -305,6 +342,7 @@ class CommentsProvider with ChangeNotifier { if (_isDisposed) return; _isLoading = false; _isLoadingMore = false; + _isQuietLoading = false; _safeNotifyListeners(); // If a refresh was scheduled during this load, execute it now @@ -340,17 +378,27 @@ class CommentsProvider with ChangeNotifier { /// Fetches the subtree rooted at [commentUri] via the getComments /// `parentRkey` parameter and merges it into the in-memory comment tree. /// This surfaces replies hidden by the per-parent sibling cap or the - /// nesting-depth cutoff of the original thread fetch. + /// nesting-depth cutoff of the original thread fetch. When the node + /// already has a [ThreadViewComment.repliesCursor] (a previous page was + /// fetched), the cursor is sent and the new page of direct replies is + /// appended instead of replacing what's already loaded. /// - /// Returns the freshly fetched subtree so callers (e.g. the focused - /// thread screen) can render it even when the node is no longer present - /// in the top-level tree. Returns null if a fetch for the same comment is - /// already in flight or the server returned no subtree. + /// Returns the merged subtree so callers (e.g. the focused thread screen) + /// can render it even when the node is no longer present in the top-level + /// tree. If a fetch for the same comment is already in flight, the + /// EXISTING future is returned, so every caller gets the real result. + /// Returns null when the server returned no/mismatched subtree (the + /// node's hasMore/cursor are cleared on an empty response so the UI stops + /// offering a load-more that can never succeed), when the response became + /// stale (tree refreshed or sort changed mid-flight), or when the + /// provider was disposed. /// + /// Throws [ArgumentError] for a malformed comment URI (programmer error). /// Throws ApiException/AuthenticationException on network or auth errors. - Future loadMoreReplies(String commentUri) async { - if (_loadingMoreReplies.contains(commentUri)) { - return null; + Future loadMoreReplies(String commentUri) { + final inFlight = _loadingMoreReplies[commentUri]; + if (inFlight != null) { + return inFlight; } // rkey is the last path segment of the comment AT-URI. Note: Uri.parse @@ -358,14 +406,36 @@ class CommentsProvider with ChangeNotifier { final segments = commentUri.split('/'); final rkey = segments.length > 1 ? segments.last : ''; if (rkey.isEmpty) { - if (kDebugMode) { - debugPrint('⚠️ loadMoreReplies: malformed comment URI: $commentUri'); - } - return null; + throw ArgumentError.value( + commentUri, + 'commentUri', + 'malformed comment AT-URI', + ); } - _loadingMoreReplies.add(commentUri); + // Register the in-flight future BEFORE starting the work: the fetch can + // fail synchronously, and _doLoadMoreReplies' cleanup must always run + // after the map entry exists or the entry would leak forever. + final completer = Completer(); + _loadingMoreReplies[commentUri] = completer.future; _safeNotifyListeners(); + completer.complete(_doLoadMoreReplies(commentUri, rkey)); + return completer.future; + } + + Future _doLoadMoreReplies( + String commentUri, + String rkey, + ) async { + // Capture staleness markers before the fetch: if the tree is wholesale + // replaced (refresh/delete) or the sort changes while we're in flight, + // this response no longer belongs to what's on screen. + final startGeneration = _treeGeneration; + final startSort = _sort; + + // Pass the stored cursor (if any) so a node with more than one page of + // direct replies advances through pages instead of refetching page 1. + final requestCursor = _findNodeByUri(commentUri)?.repliesCursor; try { final response = await _apiService.getComments( @@ -373,24 +443,104 @@ class CommentsProvider with ChangeNotifier { sort: _sort, timeframe: _timeframe, parentRkey: rkey, + cursor: requestCursor, ); - if (_isDisposed || response.comments.isEmpty) { + if (_isDisposed) { return null; } - // The response contains the subtree rooted at the requested comment as - // its sole top-level entry. The cursor paginates the parent's direct - // replies; if present there are more direct replies beyond this page. - final subtree = response.comments.first.copyWith( - hasMore: response.cursor != null, - ); + if (_treeGeneration != startGeneration || _sort != startSort) { + if (kDebugMode) { + debugPrint( + '⚠️ loadMoreReplies: discarding stale subtree for $rkey ' + '(tree refreshed or sort changed mid-flight)', + ); + } + return null; + } + + final existingNode = _findNodeByUri(commentUri); + + if (response.comments.isEmpty) { + // Nothing to load - clear the node's pagination state so the + // "load more" affordance disappears instead of spinning forever. + if (existingNode != null && + (existingNode.hasMore || existingNode.repliesCursor != null)) { + _comments = _replaceNode( + _comments, + existingNode.copyWith(hasMore: false, repliesCursor: null), + ); + } + return null; + } + + // Contract guard: the response must contain the subtree rooted at the + // requested comment as its sole top-level entry. + final fresh = response.comments.first; + if (fresh.comment.uri != commentUri) { + if (kDebugMode) { + debugPrint( + '⚠️ loadMoreReplies: response anchored at ${fresh.comment.uri}, ' + 'expected $commentUri - discarding', + ); + } + return null; + } + + // Collect URIs already in the tree BEFORE merging so we only + // initialize vote state for genuinely new comments (re-initializing + // visible ones would clobber optimistic votes). + final knownUris = {}; + if (existingNode != null) { + _collectSubtreeUris(existingNode, knownUris); + } - _comments = _replaceNode(_comments, subtree); + // The response cursor paginates this node's direct replies; if + // present there are more direct replies beyond this page. + final ThreadViewComment subtree; + if (requestCursor != null && existingNode != null) { + // Cursor page: append the new page's direct replies (deduplicated + // by URI) to the ones already loaded instead of replacing them. + final existingReplies = + existingNode.replies ?? const []; + final seenUris = existingReplies.map((r) => r.comment.uri).toSet(); + final newPage = (fresh.replies ?? const []) + .where((reply) => !seenUris.contains(reply.comment.uri)); + subtree = fresh.copyWith( + replies: [...existingReplies, ...newPage], + hasMore: response.cursor != null, + repliesCursor: response.cursor, + ); + } else { + // First page: merge with the existing node (if any) so deeper + // branches hydrated earlier survive the refetch. + final merged = + existingNode == null ? fresh : _mergeSubtree(fresh, existingNode); + subtree = merged.copyWith( + hasMore: response.cursor != null, + repliesCursor: response.cursor, + ); + } + + final updated = _replaceNode(_comments, subtree); + if (identical(updated, _comments)) { + // Node not in the top-level tree (e.g. below the depth cap when + // called from the focused thread screen) - nothing to merge, but + // the returned subtree is still useful to the caller. + if (kDebugMode) { + debugPrint( + 'ℹ️ loadMoreReplies: $commentUri not in top-level tree - ' + 'returning subtree without merging', + ); + } + } else { + _comments = updated; + } - // Initialize vote state for the newly fetched replies + // Initialize vote state only for replies we didn't already have. if (_authProvider.isAuthenticated && _voteProvider != null) { - _initializeCommentVoteState(subtree); + _initializeVoteStateForNewComments(subtree, knownUris); } if (kDebugMode) { @@ -403,19 +553,136 @@ class CommentsProvider with ChangeNotifier { return subtree; } finally { if (!_isDisposed) { - _loadingMoreReplies.remove(commentUri); + // Map.remove returns the (already-settled) future; nothing to await. + unawaited(_loadingMoreReplies.remove(commentUri)); _safeNotifyListeners(); } } } + /// Merges a freshly fetched [fresh] subtree with the [existing] version of + /// the same node already in the tree. + /// + /// Semantics: fresh data wins for node content/stats, but deeper branches + /// hydrated earlier (via nested load-more) are preserved when they are + /// absent from the fresh response only because of its depth/sibling + /// truncation - absence from a truncated response does not mean deletion. + /// When the fresh listing of a node's replies is complete (no hasMore), + /// absence DOES mean deletion and the stale children are dropped. + ThreadViewComment _mergeSubtree( + ThreadViewComment fresh, + ThreadViewComment existing, + ) { + assert( + fresh.comment.uri == existing.comment.uri, + '_mergeSubtree requires nodes with the same URI', + ); + + final freshReplies = fresh.replies; + final existingReplies = existing.replies; + + // Fresh node hit the response's depth cutoff (no replies loaded) but we + // already hydrated this branch - keep the existing branch and its + // pagination state; take the fresh node's content/stats. + if (freshReplies == null || freshReplies.isEmpty) { + if (existingReplies == null || existingReplies.isEmpty) { + return fresh; + } + return fresh.copyWith( + replies: existingReplies, + hasMore: existing.hasMore, + repliesCursor: existing.repliesCursor, + ); + } + + // Merge per-child by URI: children present in both are merged + // recursively (so grandchildren expansions survive too). + final existingByUri = { + for (final reply in existingReplies ?? const []) + reply.comment.uri: reply, + }; + final mergedReplies = [ + for (final freshChild in freshReplies) + existingByUri.containsKey(freshChild.comment.uri) + ? _mergeSubtree( + freshChild, + existingByUri.remove(freshChild.comment.uri)!, + ) + : freshChild, + ]; + + // Children we had before that are missing from a sibling-truncated + // fresh page are preserved (appended after the fresh ordering). + if (fresh.hasMore && existingByUri.isNotEmpty) { + mergedReplies.addAll(existingByUri.values); + } + + return fresh.copyWith( + replies: mergedReplies, + // Per-node reply cursors only come from earlier subtree fetches of + // that node - the fresh response doesn't carry them, so keep ours. + repliesCursor: existing.repliesCursor, + ); + } + + /// Finds the node with [uri] anywhere in the current top-level tree. + ThreadViewComment? _findNodeByUri(String uri) { + for (final node in _comments) { + final found = node.findByUri(uri); + if (found != null) { + return found; + } + } + return null; + } + + /// Collects the URIs of [node] and all its descendants into [uris]. + void _collectSubtreeUris(ThreadViewComment node, Set uris) { + uris.add(node.comment.uri); + for (final reply in node.replies ?? const []) { + _collectSubtreeUris(reply, uris); + } + } + + /// Initializes vote state for comments in [node]'s subtree whose URIs are + /// NOT in [knownUris] (mirrors the pagination pattern in loadComments: + /// never re-initialize already-visible comments, which would revert + /// optimistic votes). + void _initializeVoteStateForNewComments( + ThreadViewComment node, + Set knownUris, + ) { + if (!knownUris.contains(node.comment.uri)) { + final viewer = node.comment.viewer; + _voteProvider!.setInitialVoteState( + postUri: node.comment.uri, + voteDirection: viewer?.vote, + voteUri: viewer?.voteUri, + ); + } + for (final reply in node.replies ?? const []) { + _initializeVoteStateForNewComments(reply, knownUris); + } + } + /// Returns a copy of [nodes] with the node matching [replacement]'s URI - /// replaced by [replacement]. Leaves the tree untouched when absent. + /// replaced by [replacement]. Preserves reference identity when the node + /// is absent (returns [nodes] itself) so callers can detect a missed + /// merge via `identical`. List _replaceNode( List nodes, ThreadViewComment replacement, ) { - return nodes.map((node) => node.replaceDescendant(replacement)).toList(); + var changed = false; + final mapped = []; + for (final node in nodes) { + final result = node.replaceDescendant(replacement); + if (!identical(result, node)) { + changed = true; + } + mapped.add(result); + } + return changed ? mapped : nodes; } /// Change sort order @@ -577,32 +844,63 @@ class CommentsProvider with ChangeNotifier { debugPrint('✅ Comment created: ${response.uri}'); } - // Refresh comments to show the new comment. The AppView indexes new - // comments asynchronously (firehose), so the first refresh can race - // indexing and miss the comment we just created — retry briefly until - // it shows up. Bounded so a comment that legitimately falls outside - // the first page (deep pagination) can't loop forever. - await refreshComments(); - var attempt = 0; - while (attempt < 3 && !_treeContainsUri(_comments, response.uri)) { - attempt++; - await Future.delayed(Duration(milliseconds: 400 * attempt)); + // Surface the new comment. The AppView indexes new comments + // asynchronously (firehose lag ~1-2s), so the first fetch can race + // indexing and miss the comment we just created — retry briefly with + // backoff until it shows up. Bounded so a comment that legitimately + // falls outside the first page (deep pagination) can't loop forever. + if (parentComment == null || + _treeContainsUri(_comments, parentComment.comment.uri)) { + // Parent is visible in the top-level tree (or this is a top-level + // reply): a refresh can surface the new comment. Retries use the + // quiet path so the full list doesn't flicker into a loading state + // on every attempt. await refreshComments(); - } + var attempt = 0; + while (!_isDisposed && + attempt < _indexingRetryDelays.length && + !_treeContainsUri(_comments, response.uri)) { + await Future.delayed(_indexingRetryDelays[attempt]); + attempt++; + if (_isDisposed) { + break; + } + await loadComments(refresh: true, quiet: true); + } - // Deep replies can sit past the per-parent sibling cap or the depth - // cutoff of the top-level refresh. Pull the parent's subtree so the - // new reply is merged into the tree at its correct position. - if (parentComment != null && !_treeContainsUri(_comments, response.uri)) { - try { - await loadMoreReplies(parentComment.comment.uri); - } on Exception catch (e) { - // The comment was created successfully; failing to hydrate the - // subtree is not fatal — the reply is reachable via load-more. - if (kDebugMode) { - debugPrint('⚠️ Failed to hydrate reply subtree: $e'); + // Deep replies can still sit past the per-parent sibling cap or the + // depth cutoff of the top-level refresh. Pull the parent's subtree + // so the new reply is merged into the tree at its correct position. + if (parentComment != null && + !_isDisposed && + !_treeContainsUri(_comments, response.uri)) { + try { + await loadMoreReplies(parentComment.comment.uri); + } on Exception catch (e) { + // The comment was created successfully; failing to hydrate the + // subtree is not fatal — the reply is reachable via load-more. + if (kDebugMode) { + debugPrint('⚠️ Failed to hydrate reply subtree: $e'); + } } } + } else { + // The parent is NOT in the top-level tree (below the depth cap): + // full refreshes can never surface the new reply, so retry the + // parent's subtree fetch instead, verifying against the RETURNED + // subtree (the merge into the top-level tree is a no-op here). + var subtree = await _tryLoadReplySubtree(parentUri); + var attempt = 0; + while (!_isDisposed && + attempt < _indexingRetryDelays.length && + (subtree == null || subtree.findByUri(response.uri) == null)) { + await Future.delayed(_indexingRetryDelays[attempt]); + attempt++; + if (_isDisposed) { + break; + } + subtree = await _tryLoadReplySubtree(parentUri); + } } } on Exception catch (e) { if (kDebugMode) { @@ -614,17 +912,22 @@ class CommentsProvider with ChangeNotifier { /// Whether [nodes] (or any of their nested replies) contain a comment /// with the given [uri]. - bool _treeContainsUri(List nodes, String uri) { - for (final node in nodes) { - if (node.comment.uri == uri) { - return true; - } - final replies = node.replies; - if (replies != null && _treeContainsUri(replies, uri)) { - return true; + bool _treeContainsUri(List nodes, String uri) => + nodes.any((node) => node.findByUri(uri) != null); + + /// Fetches the subtree rooted at [parentUri], swallowing fetch errors. + /// + /// Used by the post-create verification loop: the comment was already + /// created successfully, so a failed hydration attempt is not fatal. + Future _tryLoadReplySubtree(String parentUri) async { + try { + return await loadMoreReplies(parentUri); + } on Exception catch (e) { + if (kDebugMode) { + debugPrint('⚠️ Failed to hydrate reply subtree: $e'); } + return null; } - return false; } /// Delete a comment diff --git a/lib/screens/home/focused_thread_screen.dart b/lib/screens/home/focused_thread_screen.dart index a5fa1a4..e3ce5b3 100644 --- a/lib/screens/home/focused_thread_screen.dart +++ b/lib/screens/home/focused_thread_screen.dart @@ -9,6 +9,7 @@ import '../../providers/auth_provider.dart'; import '../../providers/comments_provider.dart'; import '../../widgets/comment_card.dart'; import '../../widgets/comment_thread.dart'; +import '../../widgets/loading_error_states.dart'; import '../../widgets/status_bar_overlay.dart'; import '../compose/reply_screen.dart'; @@ -90,13 +91,26 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { final ScrollController _scrollController = ScrollController(); final GlobalKey _anchorKey = GlobalKey(); - /// Live subtree rooted at the anchor comment. + /// Local fallback subtree rooted at the anchor comment. /// /// Starts as the snapshot passed from the parent thread (which may be /// truncated by the original fetch depth) and is refreshed from the - /// server so deep replies and newly posted replies show up. + /// server so deep replies and newly posted replies show up. The build + /// method prefers the provider's live copy of the anchor node when it is + /// still present in the loaded tree; this is the fallback for anchors + /// outside it. late ThreadViewComment _thread; + /// Monotonic sequence for subtree fetches so an older in-flight response + /// can never overwrite the result of a newer one. + int _refreshSeq = 0; + + /// Whether the most recent anchor-subtree hydration failed. + /// + /// Only surfaced in the UI when the snapshot has no replies to fall back + /// on (otherwise the stale-but-usable subtree stays visible silently). + bool _hydrationFailed = false; + @override void initState() { super.initState(); @@ -112,19 +126,33 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { /// /// Also merges the subtree into the parent CommentsProvider tree (when /// the anchor is still present there), keeping the full thread in sync. - /// Failures are non-fatal: the current (possibly truncated) subtree - /// remains visible. + /// Failures are non-fatal when a snapshot with replies is already + /// visible; with an empty snapshot a retryable error state is shown. Future _refreshSubtree() async { + if (!mounted) { + return; + } final provider = context.read(); + final seq = ++_refreshSeq; try { - final subtree = await provider.loadMoreReplies(_thread.comment.uri); - if (subtree != null && mounted) { - setState(() => _thread = subtree); + final subtree = await provider.loadMoreReplies(widget.thread.comment.uri); + if (!mounted || seq != _refreshSeq) { + return; } + setState(() { + _hydrationFailed = false; + if (subtree != null) { + _thread = subtree; + } + }); } on Exception catch (e) { if (kDebugMode) { debugPrint('⚠️ Failed to refresh focused subtree: $e'); } + if (!mounted || seq != _refreshSeq) { + return; + } + setState(() => _hydrationFailed = true); } } @@ -174,6 +202,9 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { comment: comment, onSubmit: (content, facets) async { await widget.onReply(content, facets, comment); + if (!mounted) { + return; + } // Re-fetch the subtree so the new reply appears in this view await _refreshSubtree(); }, @@ -187,9 +218,12 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { Future _onLoadMoreReplies(ThreadViewComment node) async { final provider = context.read(); final messenger = ScaffoldMessenger.of(context); + // Skip the local merge when a newer anchor refresh starts while this + // fetch is in flight (the refreshed subtree supersedes it). + final seq = _refreshSeq; try { final subtree = await provider.loadMoreReplies(node.comment.uri); - if (subtree != null && mounted) { + if (subtree != null && mounted && seq == _refreshSeq) { setState(() => _thread = _thread.replaceDescendant(subtree)); } } on Exception { @@ -207,6 +241,9 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { /// Delete a comment, then refresh both the full thread and this subtree Future _onDelete(String uri) async { await context.read().deleteComment(commentUri: uri); + if (!mounted) { + return; + } await _refreshSubtree(); } @@ -232,6 +269,19 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { // Rebuilds on provider changes (e.g. per-node reply-loading spinners) final commentsProvider = context.watch(); + // Prefer the provider's live copy of the anchor node when it is still + // present in the loaded tree, so deletes, sort changes, and hydrations + // performed elsewhere reach this screen. The local snapshot is the + // fallback for anchors outside the provider's loaded tree. + var thread = _thread; + for (final root in commentsProvider.comments) { + final live = root.findByUri(widget.thread.comment.uri); + if (live != null) { + thread = live; + break; + } + } + // Calculate minimum bottom padding to allow anchor to scroll to top final screenHeight = MediaQuery.of(context).size.height; final minBottomPadding = screenHeight * 0.6; @@ -270,13 +320,14 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { // Anchor comment (the focused comment) - made prominent KeyedSubtree( key: _anchorKey, - child: _buildAnchorComment(), + child: _buildAnchorComment(thread), ), // Replies (if any) - if (_thread.replies != null && _thread.replies!.isNotEmpty) - ..._thread.replies!.map((reply) { + if (thread.replies != null && thread.replies!.isNotEmpty) + ...thread.replies!.map((reply) { return CommentThread( + key: ValueKey(reply.comment.uri), thread: reply, depth: 1, maxDepth: 6, @@ -287,14 +338,31 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { onLoadMoreReplies: _onLoadMoreReplies, loadingMoreReplies: commentsProvider.loadingMoreReplies, - ancestors: [_thread], + ancestors: [thread], onDelete: _onDelete, ); }), - // Empty state if no replies - if (_thread.replies == null || _thread.replies!.isEmpty) - _buildNoReplies(), + // More direct replies to the anchor beyond this page + if (thread.hasMore && + !_collapsedComments.contains(thread.comment.uri)) + LoadMoreRepliesButton( + depth: 0, + isLoading: commentsProvider.loadingMoreReplies + .contains(thread.comment.uri), + onTap: () => _onLoadMoreReplies(thread), + ), + + // Empty state (or retryable error) if no replies loaded + if (thread.replies == null || thread.replies!.isEmpty) ...[ + if (_hydrationFailed) + InlineError( + message: 'Could not load replies. Please try again.', + onRetry: _refreshSubtree, + ) + else if (!thread.hasMore) + _buildNoReplies(), + ], // Bottom padding to allow anchor to scroll to top SizedBox(height: minBottomPadding), @@ -324,7 +392,7 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { } /// Build the anchor comment (the focused comment) with prominent styling - Widget _buildAnchorComment() { + Widget _buildAnchorComment(ThreadViewComment thread) { // Note: CommentCard has its own Consumer for vote state return Container( decoration: BoxDecoration( @@ -338,12 +406,12 @@ class _FocusedThreadBodyState extends State<_FocusedThreadBody> { ), ), child: CommentCard( - comment: _thread.comment, - onTap: () => _openReplyScreen(_thread), - onLongPress: () => _toggleCollapsed(_thread.comment.uri), - isCollapsed: _collapsedComments.contains(_thread.comment.uri), - collapsedCount: _collapsedComments.contains(_thread.comment.uri) - ? _thread.comment.stats.replyCount + comment: thread.comment, + onTap: () => _openReplyScreen(thread), + onLongPress: () => _toggleCollapsed(thread.comment.uri), + isCollapsed: _collapsedComments.contains(thread.comment.uri), + collapsedCount: _collapsedComments.contains(thread.comment.uri) + ? thread.comment.stats.replyCount : 0, onDelete: _onDelete, ), diff --git a/lib/screens/home/post_detail_screen.dart b/lib/screens/home/post_detail_screen.dart index d62f631..fb2ea1a 100644 --- a/lib/screens/home/post_detail_screen.dart +++ b/lib/screens/home/post_detail_screen.dart @@ -52,6 +52,25 @@ class PostDetailScreen extends StatefulWidget { /// When true, skips initial comment load since we know there are no comments final bool isOptimistic; + /// The comment count to display in the comments header. + /// + /// The server-side [serverCount] includes comments the viewer never sees + /// (deleted, blocked, filtered). When the thread resolved successfully but + /// nothing is renderable (not loading, no error, no comments, no further + /// pages), returns 0 so the header shows its empty state instead of a + /// count over an empty list. In every other state the server count stands. + @visibleForTesting + static int displayedCommentCount({ + required int serverCount, + required bool isLoading, + required bool hasError, + required bool hasComments, + required bool hasMore, + }) { + final resolvedEmpty = !isLoading && !hasError && !hasComments && !hasMore; + return resolvedEmpty ? 0 : serverCount; + } + @override State createState() => _PostDetailScreenState(); } @@ -908,16 +927,17 @@ class _PostDetailScreenState extends State { CommentsHeader( key: _commentsHeaderKey, commentCount: - (!isLoading && - error == null && - comments.isEmpty && - !commentsProvider.hasMore) - ? 0 - : widget - .post - .post - .stats - .commentCount, + PostDetailScreen.displayedCommentCount( + serverCount: widget + .post + .post + .stats + .commentCount, + isLoading: isLoading, + hasError: error != null, + hasComments: comments.isNotEmpty, + hasMore: commentsProvider.hasMore, + ), currentSort: commentsProvider.sort, onSortChanged: _onSortChanged, ), diff --git a/lib/widgets/comment_card.dart b/lib/widgets/comment_card.dart index 84bf523..a26c1ff 100644 --- a/lib/widgets/comment_card.dart +++ b/lib/widgets/comment_card.dart @@ -171,7 +171,7 @@ class _CommentCardState extends State { leftPadding, 12, 16, - isCollapsed || comment.isDeleted ? 12 : 8, + isCollapsed || comment.isTombstoned ? 12 : 8, ), child: Column( crossAxisAlignment: CrossAxisAlignment.start, @@ -179,8 +179,11 @@ class _CommentCardState extends State { // Author info row Row( children: [ - // Author avatar and handle (or placeholder for deleted) - if (comment.isDeleted || author == null) + // Author avatar and handle (or placeholder for + // deleted). The author == null arm is already + // covered by isTombstoned; spelling it out lets + // Dart promote author to non-null below. + if (author == null || comment.isTombstoned) // Show deletion reason as placeholder Text( comment.deletionReason == 'moderator' @@ -238,7 +241,7 @@ class _CommentCardState extends State { ), // Only show content and actions when expanded (skip for deleted) - if (!isCollapsed && !comment.isDeleted) ...[ + if (!isCollapsed && !comment.isTombstoned) ...[ const SizedBox(height: 8), // Comment content diff --git a/lib/widgets/comment_thread.dart b/lib/widgets/comment_thread.dart index c3310d2..8250e19 100644 --- a/lib/widgets/comment_thread.dart +++ b/lib/widgets/comment_thread.dart @@ -106,6 +106,10 @@ class CommentThread extends StatelessWidget { children: thread.replies!.map((reply) { return CommentThread( + // Keyed by URI: CommentCard is stateful, so merges + // that reorder siblings must not re-associate state + // by index. + key: ValueKey(reply.comment.uri), thread: reply, depth: depth + 1, maxDepth: maxDepth, @@ -218,7 +222,20 @@ class CommentThread extends StatelessWidget { _buildContinueThreadLink(context, replyCount), // Show "Load more replies" button if there are more (and not collapsed) - if (thread.hasMore && !isCollapsed) _buildLoadMoreButton(context), + if (thread.hasMore && !isCollapsed) + LoadMoreRepliesButton( + depth: depth, + isLoading: loadingMoreReplies.contains(thread.comment.uri), + onTap: () { + if (onLoadMoreReplies != null) { + onLoadMoreReplies!(thread); + } else { + if (kDebugMode) { + debugPrint('Load more replies tapped (no handler provided)'); + } + } + }, + ), ], ); } @@ -279,11 +296,33 @@ class CommentThread extends StatelessWidget { ); } - /// Builds the "Load more replies" button - Widget _buildLoadMoreButton(BuildContext context) { +} + +/// "Load more replies" button shared by [CommentThread] (nested levels) +/// and the focused thread screen (the anchor's own reply pagination). +/// +/// Shows a spinner and disables taps while [isLoading] is true. +class LoadMoreRepliesButton extends StatelessWidget { + const LoadMoreRepliesButton({ + required this.depth, + this.isLoading = false, + this.onTap, + super.key, + }); + + /// Nesting depth of the parent comment (controls left alignment) + final int depth; + + /// Whether a "load more replies" fetch is in flight for this comment + final bool isLoading; + + /// Callback when the button is tapped (ignored while loading) + final VoidCallback? onTap; + + @override + Widget build(BuildContext context) { // Calculate left padding based on depth (align with replies) final leftPadding = 16.0 + ((depth + 1) * 12.0); - final isLoading = loadingMoreReplies.contains(thread.comment.uri); return Container( padding: EdgeInsets.fromLTRB(leftPadding, 8, 16, 8), @@ -291,19 +330,7 @@ class CommentThread extends StatelessWidget { border: Border(bottom: BorderSide(color: AppColors.border)), ), child: InkWell( - onTap: isLoading - ? null - : () { - if (onLoadMoreReplies != null) { - onLoadMoreReplies!(thread); - } else { - if (kDebugMode) { - debugPrint( - 'Load more replies tapped (no handler provided)', - ); - } - } - }, + onTap: isLoading ? null : onTap, child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Row( diff --git a/test/models/comment_test.dart b/test/models/comment_test.dart index 2d684ca..f0e7807 100644 --- a/test/models/comment_test.dart +++ b/test/models/comment_test.dart @@ -1,4 +1,5 @@ import 'package:coves_flutter/models/comment.dart'; +import 'package:coves_flutter/models/post.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { @@ -165,6 +166,90 @@ void main() { final thread = ThreadViewComment.fromJson(json); expect(thread.hasMore, false); + expect(thread.repliesCursor, isNull); + }); + + test('replies list is unmodifiable', () { + final node = _node('parent', replies: [_node('child')]); + + expect( + () => node.replies!.add(_node('other')), + throwsUnsupportedError, + ); + }); + + group('copyWith', () { + test('keeps repliesCursor when not provided', () { + final node = _node('a', repliesCursor: 'cursor-1'); + + final copy = node.copyWith(hasMore: true); + + expect(copy.repliesCursor, 'cursor-1'); + expect(copy.hasMore, true); + }); + + test('sets and clears repliesCursor explicitly', () { + final node = _node('a', repliesCursor: 'cursor-1'); + + expect(node.copyWith(repliesCursor: 'cursor-2').repliesCursor, + 'cursor-2'); + // Explicit null clears the cursor (sentinel distinguishes this + // from "not provided"). + expect(node.copyWith(repliesCursor: null).repliesCursor, isNull); + }); + }); + + group('replaceDescendant', () { + test('replaces a deep descendant and preserves sibling identity', () { + final grandchild = _node('c'); + final child = _node('b', replies: [grandchild]); + final sibling = _node('sibling'); + final root = _node('a', replies: [child, sibling]); + + final replacement = _node('c', replies: [_node('d')]); + final result = root.replaceDescendant(replacement); + + expect(identical(result, root), isFalse); + expect( + result.replies!.first.replies!.single.replies!.single.comment.uri, + 'd', + ); + // Untouched sibling branch keeps reference identity. + expect(identical(result.replies![1], sibling), isTrue); + }); + + test('returns identical root when the target is absent', () { + final root = _node( + 'a', + replies: [ + _node('b', replies: [_node('c')]), + ], + ); + + final result = root.replaceDescendant(_node('not-in-tree')); + + expect(identical(result, root), isTrue); + }); + }); + + group('findByUri', () { + test('finds a deeply nested node', () { + final target = _node('c'); + final root = _node( + 'a', + replies: [ + _node('b', replies: [target]), + ], + ); + + expect(identical(root.findByUri('c'), target), isTrue); + }); + + test('returns null when absent', () { + final root = _node('a', replies: [_node('b')]); + + expect(root.findByUri('missing'), isNull); + }); }); }); @@ -496,6 +581,47 @@ void main() { expect(comment.deletionReason, 'moderator'); }); + test('isTombstoned is true for deleted or author-less comments', () { + final deleted = CommentView( + uri: 'at://did:plc:test/comment/1', + cid: 'cid1', + isDeleted: true, + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + post: CommentRef(uri: 'at://did:plc:test/post/123', cid: 'post-cid'), + stats: const CommentStats(), + ); + expect(deleted.isTombstoned, isTrue); + + final normal = CommentView( + uri: 'at://did:plc:test/comment/2', + cid: 'cid2', + record: const CommentRecord(content: 'hello'), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + author: AuthorView(did: 'did:plc:author', handle: 'test.user'), + post: CommentRef(uri: 'at://did:plc:test/post/123', cid: 'post-cid'), + stats: const CommentStats(), + ); + expect(normal.isTombstoned, isFalse); + }); + + test('asserts that non-deleted comments have an author', () { + expect( + () => CommentView( + uri: 'at://did:plc:test/comment/1', + cid: 'cid1', + record: const CommentRecord(content: 'hello'), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + // author omitted while not deleted violates the invariant + post: CommentRef(uri: 'at://did:plc:test/post/123', cid: 'post-cid'), + stats: const CommentStats(), + ), + throwsAssertionError, + ); + }); + test('should parse deleted comment in thread', () { final json = { 'comment': { @@ -689,3 +815,25 @@ void main() { }); }); } + +/// Builds a minimal ThreadViewComment node for tree-manipulation tests. +ThreadViewComment _node( + String uri, { + List? replies, + String? repliesCursor, +}) { + return ThreadViewComment( + comment: CommentView( + uri: uri, + cid: 'cid-$uri', + record: const CommentRecord(content: 'content'), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + author: AuthorView(did: 'did:plc:author', handle: 'test.user'), + post: CommentRef(uri: 'at://did:plc:test/post/123', cid: 'post-cid'), + stats: const CommentStats(), + ), + replies: replies, + repliesCursor: repliesCursor, + ); +} diff --git a/test/providers/comments_provider_test.dart b/test/providers/comments_provider_test.dart index bf4f5ef..d4cad22 100644 --- a/test/providers/comments_provider_test.dart +++ b/test/providers/comments_provider_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:coves_flutter/models/comment.dart'; import 'package:coves_flutter/models/post.dart'; import 'package:coves_flutter/providers/auth_provider.dart'; @@ -1167,6 +1169,12 @@ void main() { apiService: mockApiService, voteProvider: mockVoteProvider, commentService: mockCommentService, + // Keep the indexing-lag retry schedule but skip the real waits. + indexingRetryDelays: const [ + Duration.zero, + Duration.zero, + Duration.zero, + ], ); }); @@ -1526,6 +1534,141 @@ void main() { }, ); + test( + 'should stop retrying as soon as the created comment is indexed', + () async { + await providerWithCommentService.loadComments(refresh: true); + + when( + mockCommentService.createComment( + rootUri: anyNamed('rootUri'), + rootCid: anyNamed('rootCid'), + parentUri: anyNamed('parentUri'), + parentCid: anyNamed('parentCid'), + content: anyNamed('content'), + ), + ).thenAnswer( + (_) async => const CreateCommentResponse( + uri: 'at://did:plc:test/comment/abc', + cid: 'cid123', + ), + ); + + // First post-create refresh misses the comment (indexing lag); + // the first retry finds it. No further retries may happen. + var fetchCount = 0; + final indexedResponse = CommentsResponse( + post: {}, + comments: [ + _createMockThreadComment('at://did:plc:test/comment/abc'), + _createMockThreadComment('comment1'), + ], + ); + final notIndexedResponse = CommentsResponse( + post: {}, + comments: [_createMockThreadComment('comment1')], + ); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).thenAnswer((_) async { + fetchCount++; + // Call 1 is the post-create refresh; call 2 is retry 1. + return fetchCount >= 2 ? indexedResponse : notIndexedResponse; + }); + + await providerWithCommentService.createComment( + content: 'Test comment', + ); + + // Refresh + exactly one retry - the remaining schedule is skipped. + expect(fetchCount, 2); + }, + ); + + test( + 'should retry the parent subtree (not full refreshes) when the ' + 'parent is below the top-level depth cap', + () async { + await providerWithCommentService.loadComments(refresh: true); + + const deepParentUri = + 'at://did:plc:author/social.coves.community.comment/deeprkey'; + const replyUri = 'at://did:plc:test/comment/deep-reply'; + + when( + mockCommentService.createComment( + rootUri: anyNamed('rootUri'), + rootCid: anyNamed('rootCid'), + parentUri: anyNamed('parentUri'), + parentCid: anyNamed('parentCid'), + content: anyNamed('content'), + ), + ).thenAnswer( + (_) async => const CreateCommentResponse( + uri: replyUri, + cid: 'cid-deep', + ), + ); + + // First subtree fetch misses the reply (indexing lag); the first + // retry contains it. + var subtreeFetchCount = 0; + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(equals('deeprkey'), named: 'parentRkey'), + ), + ).thenAnswer((_) async { + subtreeFetchCount++; + return CommentsResponse( + post: {}, + comments: [ + _createMockThreadCommentWithViewer( + uri: deepParentUri, + replies: + subtreeFetchCount >= 2 + ? [_createMockThreadComment(replyUri)] + : [], + ), + ], + ); + }); + + // The parent is NOT in the top-level tree (below the depth cap). + await providerWithCommentService.createComment( + content: 'Deep nested reply', + parentComment: _createMockThreadComment(deepParentUri), + ); + + // Subtree fetched twice: initial attempt + one retry. + expect(subtreeFetchCount, 2); + + // No full-tree refreshes beyond the initial load: they can never + // surface a reply whose parent sits below the depth cap. + verify( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).called(1); + }, + ); + test('should rethrow exception from CommentService', () async { await providerWithCommentService.loadComments(refresh: true); @@ -1660,11 +1803,330 @@ void main() { expect(commentsProvider.loadingMoreReplies, isEmpty); }); - test('should keep hasMore when the subtree response has a cursor', + test( + 'should store the cursor, send it on the next page, and append ' + 'deduplicated replies', + () async { + await commentsProvider.loadComments(refresh: true); + + // Page 1: cursor present -> more direct replies exist. + final page1 = _createMockThreadCommentWithViewer( + uri: parentUri, + replies: [_createMockThreadComment('child-1')], + ); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: argThat(isNull, named: 'cursor'), + parentRkey: argThat(equals('parentrkey1'), named: 'parentRkey'), + ), + ).thenAnswer( + (_) async => CommentsResponse( + post: {}, + comments: [page1], + cursor: 'replies-page-2', + ), + ); + + await commentsProvider.loadMoreReplies(parentUri); + + var merged = commentsProvider.comments.single; + expect(merged.hasMore, isTrue); + expect(merged.repliesCursor, 'replies-page-2'); + expect(merged.replies, hasLength(1)); + + // Page 2: server may re-send an overlapping reply; it must be + // deduplicated. No cursor -> last page. + final page2 = _createMockThreadCommentWithViewer( + uri: parentUri, + replies: [ + _createMockThreadComment('child-1'), // duplicate + _createMockThreadComment('child-2'), + ], + ); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: 'replies-page-2', + parentRkey: argThat(equals('parentrkey1'), named: 'parentRkey'), + ), + ).thenAnswer( + (_) async => CommentsResponse(post: {}, comments: [page2]), + ); + + await commentsProvider.loadMoreReplies(parentUri); + + // The stored cursor must have been sent on the second fetch. + verify( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: 'replies-page-2', + parentRkey: argThat(equals('parentrkey1'), named: 'parentRkey'), + ), + ).called(1); + + merged = commentsProvider.comments.single; + // Appended without duplicating child-1, pagination exhausted. + expect( + merged.replies!.map((r) => r.comment.uri), + ['child-1', 'child-2'], + ); + expect(merged.hasMore, isFalse); + expect(merged.repliesCursor, isNull); + }, + ); + + test( + 'should preserve deeper hydrated branches when re-fetching an ' + 'ancestor subtree', + () async { + const childUri = + 'at://did:plc:author/social.coves.community.comment/childrkey1'; + await commentsProvider.loadComments(refresh: true); + + // Hydrate the parent: one child that itself has unloaded replies. + final parentSubtree = _createMockThreadCommentWithViewer( + uri: parentUri, + replies: [ + ThreadViewComment( + comment: _createMockThreadComment(childUri).comment, + hasMore: true, + ), + ], + ); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(equals('parentrkey1'), named: 'parentRkey'), + ), + ).thenAnswer( + (_) async => CommentsResponse(post: {}, comments: [parentSubtree]), + ); + await commentsProvider.loadMoreReplies(parentUri); + + // Hydrate the child deep: child -> grandchild. + final childSubtree = _createMockThreadCommentWithViewer( + uri: childUri, + replies: [_createMockThreadComment('grandchild-1')], + ); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(equals('childrkey1'), named: 'parentRkey'), + ), + ).thenAnswer( + (_) async => CommentsResponse(post: {}, comments: [childSubtree]), + ); + await commentsProvider.loadMoreReplies(childUri); + + expect( + commentsProvider.comments.single.replies!.single.replies!.single + .comment.uri, + 'grandchild-1', + ); + + // Re-hydrate the ancestor: the fresh response truncates the child + // at the depth cutoff (no replies loaded). The child's hydrated + // expansion must survive the merge. + await commentsProvider.loadMoreReplies(parentUri); + + final child = commentsProvider.comments.single.replies!.single; + expect(child.comment.uri, childUri); + expect(child.replies, isNotNull); + expect(child.replies!.single.comment.uri, 'grandchild-1'); + }, + ); + + test('should return the existing future for duplicate in-flight calls', + () async { + await commentsProvider.loadComments(refresh: true); + + final completer = Completer(); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(isNotNull, named: 'parentRkey'), + ), + ).thenAnswer((_) => completer.future); + + final first = commentsProvider.loadMoreReplies(parentUri); + expect(commentsProvider.loadingMoreReplies, contains(parentUri)); + + final second = commentsProvider.loadMoreReplies(parentUri); + expect(identical(first, second), isTrue); + + completer.complete( + CommentsResponse( + post: {}, + comments: [ + _createMockThreadCommentWithViewer( + uri: parentUri, + replies: [_createMockThreadComment('child-1')], + ), + ], + ), + ); + + final results = await Future.wait([first, second]); + expect(results[0], isNotNull); + expect(identical(results[0], results[1]), isTrue); + expect(commentsProvider.loadingMoreReplies, isEmpty); + + // Only one network call despite two callers. + verify( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(isNotNull, named: 'parentRkey'), + ), + ).called(1); + }); + + test( + 'should clear hasMore and cursor when the subtree response is empty', + () async { + await commentsProvider.loadComments(refresh: true); + expect(commentsProvider.comments.single.hasMore, isTrue); + + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(isNotNull, named: 'parentRkey'), + ), + ).thenAnswer((_) async => CommentsResponse(post: {}, comments: [])); + + final result = await commentsProvider.loadMoreReplies(parentUri); + + expect(result, isNull); + // The load-more affordance must disappear rather than spin forever. + expect(commentsProvider.comments.single.hasMore, isFalse); + expect(commentsProvider.comments.single.repliesCursor, isNull); + }, + ); + + test('should throw ArgumentError for a malformed comment URI', () { + expect( + () => commentsProvider.loadMoreReplies('no-slashes'), + throwsArgumentError, + ); + expect(commentsProvider.loadingMoreReplies, isEmpty); + }); + + test( + 'should discard a response anchored at a different comment', + () async { + await commentsProvider.loadComments(refresh: true); + + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(isNotNull, named: 'parentRkey'), + ), + ).thenAnswer( + (_) async => CommentsResponse( + post: {}, + comments: [ + _createMockThreadCommentWithViewer( + uri: 'some-other-comment', + replies: [_createMockThreadComment('child-1')], + ), + ], + ), + ); + + final result = await commentsProvider.loadMoreReplies(parentUri); + + expect(result, isNull); + // Tree untouched by the mismatched response. + expect(commentsProvider.comments.single.replies, isNull); + expect(commentsProvider.comments.single.hasMore, isTrue); + }, + ); + + test( + 'should discard a subtree response that completes after a refresh', + () async { + await commentsProvider.loadComments(refresh: true); + + final completer = Completer(); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(isNotNull, named: 'parentRkey'), + ), + ).thenAnswer((_) => completer.future); + + final pending = commentsProvider.loadMoreReplies(parentUri); + + // A full refresh replaces the tree while the subtree fetch is + // still in flight. + await commentsProvider.refreshComments(); + + completer.complete( + CommentsResponse( + post: {}, + comments: [ + _createMockThreadCommentWithViewer( + uri: parentUri, + replies: [_createMockThreadComment('stale-child')], + ), + ], + ), + ); + + final result = await pending; + + // Stale response: not merged, not returned. + expect(result, isNull); + expect(commentsProvider.comments.single.replies, isNull); + }, + ); + + test('should not clobber vote state of already-visible comments', () async { await commentsProvider.loadComments(refresh: true); - final subtree = _createMockThreadCommentWithViewer( + // First hydration: parent + child-1. + final page1 = _createMockThreadCommentWithViewer( uri: parentUri, replies: [_createMockThreadComment('child-1')], ); @@ -1678,16 +2140,56 @@ void main() { parentRkey: argThat(isNotNull, named: 'parentRkey'), ), ).thenAnswer( - (_) async => CommentsResponse( - post: {}, - comments: [subtree], - cursor: 'more-direct-replies', - ), + (_) async => CommentsResponse(post: {}, comments: [page1]), ); + await commentsProvider.loadMoreReplies(parentUri); + clearInteractions(mockVoteProvider); + // Second hydration adds child-2; parent and child-1 are already + // visible and must NOT be re-initialized (that would revert + // optimistic votes). + final page2 = _createMockThreadCommentWithViewer( + uri: parentUri, + replies: [ + _createMockThreadComment('child-1'), + _createMockThreadComment('child-2'), + ], + ); + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + parentRkey: argThat(isNotNull, named: 'parentRkey'), + ), + ).thenAnswer( + (_) async => CommentsResponse(post: {}, comments: [page2]), + ); await commentsProvider.loadMoreReplies(parentUri); - expect(commentsProvider.comments.single.hasMore, isTrue); + verify( + mockVoteProvider.setInitialVoteState( + postUri: 'child-2', + voteDirection: anyNamed('voteDirection'), + voteUri: anyNamed('voteUri'), + ), + ).called(1); + verifyNever( + mockVoteProvider.setInitialVoteState( + postUri: parentUri, + voteDirection: anyNamed('voteDirection'), + voteUri: anyNamed('voteUri'), + ), + ); + verifyNever( + mockVoteProvider.setInitialVoteState( + postUri: 'child-1', + voteDirection: anyNamed('voteDirection'), + voteUri: anyNamed('voteUri'), + ), + ); }); test('should propagate fetch errors and clear loading state', () async { diff --git a/test/router/post_route_test.dart b/test/router/post_route_test.dart index 58950c1..987541a 100644 --- a/test/router/post_route_test.dart +++ b/test/router/post_route_test.dart @@ -119,7 +119,14 @@ void main() { ); expect(loader.postUri, 'foo%zz'); expect(find.byType(NotFoundError), findsOneWidget); - expect(find.text('Post Not Found'), findsOneWidget); + // Title renders in both the app bar (9df9035) and the body. + expect( + find.descendant( + of: find.byType(AppBar), + matching: find.text('Post Not Found'), + ), + findsOneWidget, + ); }, ); }); diff --git a/test/test_helpers/test_mocks.dart b/test/test_helpers/test_mocks.dart new file mode 100644 index 0000000..e3deb91 --- /dev/null +++ b/test/test_helpers/test_mocks.dart @@ -0,0 +1,22 @@ +// Shared mockito mocks for widget tests. +// +// Widget tests previously imported generated mocks from +// test/providers/comments_provider_test.mocks.dart, coupling them to another +// test file's codegen. This helper owns the @GenerateMocks annotation for the +// interfaces shared across widget tests; import `test_mocks.mocks.dart` +// (re-exported here) instead of reaching into test/providers. +// +// Regenerate with: +// dart run build_runner build --delete-conflicting-outputs + +import 'package:coves_flutter/providers/auth_provider.dart'; +import 'package:coves_flutter/providers/vote_provider.dart'; +import 'package:coves_flutter/services/comment_service.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:mockito/annotations.dart'; + +export 'test_mocks.mocks.dart'; + +@GenerateMocks([AuthProvider, CovesApiService, VoteProvider, CommentService]) +// ignore: unreachable_from_main +void main() {} diff --git a/test/test_helpers/test_mocks.mocks.dart b/test/test_helpers/test_mocks.mocks.dart new file mode 100644 index 0000000..ffe79cc --- /dev/null +++ b/test/test_helpers/test_mocks.mocks.dart @@ -0,0 +1,846 @@ +// Mocks generated by Mockito 5.4.6 from annotations +// in coves_flutter/test/test_helpers/test_mocks.dart. +// Do not manually edit this file. + +// ignore_for_file: no_leading_underscores_for_library_prefixes +import 'dart:async' as _i9; +import 'dart:typed_data' as _i13; +import 'dart:ui' as _i10; + +import 'package:coves_flutter/models/comment.dart' as _i3; +import 'package:coves_flutter/models/community.dart' as _i4; +import 'package:coves_flutter/models/post.dart' as _i2; +import 'package:coves_flutter/models/post_get_result.dart' as _i11; +import 'package:coves_flutter/models/user_profile.dart' as _i5; +import 'package:coves_flutter/providers/auth_provider.dart' as _i8; +import 'package:coves_flutter/providers/vote_provider.dart' as _i14; +import 'package:coves_flutter/services/comment_service.dart' as _i7; +import 'package:coves_flutter/services/coves_api_service.dart' as _i6; +import 'package:mockito/mockito.dart' as _i1; +import 'package:mockito/src/dummies.dart' as _i12; + +// ignore_for_file: type=lint +// ignore_for_file: avoid_redundant_argument_values +// ignore_for_file: avoid_setters_without_getters +// ignore_for_file: comment_references +// ignore_for_file: deprecated_member_use +// ignore_for_file: deprecated_member_use_from_same_package +// ignore_for_file: implementation_imports +// ignore_for_file: invalid_use_of_visible_for_testing_member +// ignore_for_file: must_be_immutable +// ignore_for_file: prefer_const_constructors +// ignore_for_file: unnecessary_parenthesis +// ignore_for_file: camel_case_types +// ignore_for_file: subtype_of_sealed_class +// ignore_for_file: invalid_use_of_internal_member + +class _FakeTimelineResponse_0 extends _i1.SmartFake + implements _i2.TimelineResponse { + _FakeTimelineResponse_0(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCommentsResponse_1 extends _i1.SmartFake + implements _i3.CommentsResponse { + _FakeCommentsResponse_1(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCommunitiesResponse_2 extends _i1.SmartFake + implements _i4.CommunitiesResponse { + _FakeCommunitiesResponse_2(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCommunityView_3 extends _i1.SmartFake implements _i4.CommunityView { + _FakeCommunityView_3(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCreatePostResponse_4 extends _i1.SmartFake + implements _i4.CreatePostResponse { + _FakeCreatePostResponse_4(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCreateCommunityResponse_5 extends _i1.SmartFake + implements _i4.CreateCommunityResponse { + _FakeCreateCommunityResponse_5(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeUserProfile_6 extends _i1.SmartFake implements _i5.UserProfile { + _FakeUserProfile_6(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeActorCommentsResponse_7 extends _i1.SmartFake + implements _i3.ActorCommentsResponse { + _FakeActorCommentsResponse_7(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeUpdateProfileResponse_8 extends _i1.SmartFake + implements _i6.UpdateProfileResponse { + _FakeUpdateProfileResponse_8(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCreateCommentResponse_9 extends _i1.SmartFake + implements _i7.CreateCommentResponse { + _FakeCreateCommentResponse_9(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +/// A class which mocks [AuthProvider]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockAuthProvider extends _i1.Mock implements _i8.AuthProvider { + MockAuthProvider() { + _i1.throwOnMissingStub(this); + } + + @override + bool get isAuthenticated => + (super.noSuchMethod( + Invocation.getter(#isAuthenticated), + returnValue: false, + ) + as bool); + + @override + bool get isLoading => + (super.noSuchMethod(Invocation.getter(#isLoading), returnValue: false) + as bool); + + @override + bool get hasListeners => + (super.noSuchMethod(Invocation.getter(#hasListeners), returnValue: false) + as bool); + + @override + _i9.Future getAccessToken() => + (super.noSuchMethod( + Invocation.method(#getAccessToken, []), + returnValue: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future initialize() => + (super.noSuchMethod( + Invocation.method(#initialize, []), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future signIn(String? handle) => + (super.noSuchMethod( + Invocation.method(#signIn, [handle]), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future signOut() => + (super.noSuchMethod( + Invocation.method(#signOut, []), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future refreshToken() => + (super.noSuchMethod( + Invocation.method(#refreshToken, []), + returnValue: _i9.Future.value(false), + ) + as _i9.Future); + + @override + void clearError() => super.noSuchMethod( + Invocation.method(#clearError, []), + returnValueForMissingStub: null, + ); + + @override + void addListener(_i10.VoidCallback? listener) => super.noSuchMethod( + Invocation.method(#addListener, [listener]), + returnValueForMissingStub: null, + ); + + @override + void removeListener(_i10.VoidCallback? listener) => super.noSuchMethod( + Invocation.method(#removeListener, [listener]), + returnValueForMissingStub: null, + ); + + @override + void dispose() => super.noSuchMethod( + Invocation.method(#dispose, []), + returnValueForMissingStub: null, + ); + + @override + void notifyListeners() => super.noSuchMethod( + Invocation.method(#notifyListeners, []), + returnValueForMissingStub: null, + ); +} + +/// A class which mocks [CovesApiService]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockCovesApiService extends _i1.Mock implements _i6.CovesApiService { + MockCovesApiService() { + _i1.throwOnMissingStub(this); + } + + @override + _i9.Future<_i2.TimelineResponse> getTimeline({ + String? sort = 'hot', + String? timeframe, + int? limit = 15, + String? cursor, + }) => + (super.noSuchMethod( + Invocation.method(#getTimeline, [], { + #sort: sort, + #timeframe: timeframe, + #limit: limit, + #cursor: cursor, + }), + returnValue: _i9.Future<_i2.TimelineResponse>.value( + _FakeTimelineResponse_0( + this, + Invocation.method(#getTimeline, [], { + #sort: sort, + #timeframe: timeframe, + #limit: limit, + #cursor: cursor, + }), + ), + ), + ) + as _i9.Future<_i2.TimelineResponse>); + + @override + _i9.Future<_i2.TimelineResponse> getDiscover({ + String? sort = 'hot', + String? timeframe, + int? limit = 15, + String? cursor, + }) => + (super.noSuchMethod( + Invocation.method(#getDiscover, [], { + #sort: sort, + #timeframe: timeframe, + #limit: limit, + #cursor: cursor, + }), + returnValue: _i9.Future<_i2.TimelineResponse>.value( + _FakeTimelineResponse_0( + this, + Invocation.method(#getDiscover, [], { + #sort: sort, + #timeframe: timeframe, + #limit: limit, + #cursor: cursor, + }), + ), + ), + ) + as _i9.Future<_i2.TimelineResponse>); + + @override + _i9.Future<_i2.TimelineResponse> getCommunityFeed({ + required String? community, + String? sort = 'hot', + String? timeframe, + int? limit = 15, + String? cursor, + }) => + (super.noSuchMethod( + Invocation.method(#getCommunityFeed, [], { + #community: community, + #sort: sort, + #timeframe: timeframe, + #limit: limit, + #cursor: cursor, + }), + returnValue: _i9.Future<_i2.TimelineResponse>.value( + _FakeTimelineResponse_0( + this, + Invocation.method(#getCommunityFeed, [], { + #community: community, + #sort: sort, + #timeframe: timeframe, + #limit: limit, + #cursor: cursor, + }), + ), + ), + ) + as _i9.Future<_i2.TimelineResponse>); + + @override + _i9.Future<_i3.CommentsResponse> getComments({ + required String? postUri, + String? sort = 'hot', + String? timeframe, + int? depth = 10, + int? limit = 50, + String? cursor, + String? parentRkey, + }) => + (super.noSuchMethod( + Invocation.method(#getComments, [], { + #postUri: postUri, + #sort: sort, + #timeframe: timeframe, + #depth: depth, + #limit: limit, + #cursor: cursor, + #parentRkey: parentRkey, + }), + returnValue: _i9.Future<_i3.CommentsResponse>.value( + _FakeCommentsResponse_1( + this, + Invocation.method(#getComments, [], { + #postUri: postUri, + #sort: sort, + #timeframe: timeframe, + #depth: depth, + #limit: limit, + #cursor: cursor, + #parentRkey: parentRkey, + }), + ), + ), + ) + as _i9.Future<_i3.CommentsResponse>); + + @override + _i9.Future> getPosts({ + required List? uris, + }) => + (super.noSuchMethod( + Invocation.method(#getPosts, [], {#uris: uris}), + returnValue: _i9.Future>.value( + <_i11.PostGetResult>[], + ), + ) + as _i9.Future>); + + @override + _i9.Future<_i11.PostGetResult> getPost(String? uri) => + (super.noSuchMethod( + Invocation.method(#getPost, [uri]), + returnValue: _i9.Future<_i11.PostGetResult>.value( + _i12.dummyValue<_i11.PostGetResult>( + this, + Invocation.method(#getPost, [uri]), + ), + ), + ) + as _i9.Future<_i11.PostGetResult>); + + @override + _i9.Future<_i4.CommunitiesResponse> listCommunities({ + int? limit = 50, + String? cursor, + String? sort = 'popular', + bool? subscribed, + }) => + (super.noSuchMethod( + Invocation.method(#listCommunities, [], { + #limit: limit, + #cursor: cursor, + #sort: sort, + #subscribed: subscribed, + }), + returnValue: _i9.Future<_i4.CommunitiesResponse>.value( + _FakeCommunitiesResponse_2( + this, + Invocation.method(#listCommunities, [], { + #limit: limit, + #cursor: cursor, + #sort: sort, + #subscribed: subscribed, + }), + ), + ), + ) + as _i9.Future<_i4.CommunitiesResponse>); + + @override + _i9.Future<_i4.CommunityView> getCommunity({required String? community}) => + (super.noSuchMethod( + Invocation.method(#getCommunity, [], {#community: community}), + returnValue: _i9.Future<_i4.CommunityView>.value( + _FakeCommunityView_3( + this, + Invocation.method(#getCommunity, [], {#community: community}), + ), + ), + ) + as _i9.Future<_i4.CommunityView>); + + @override + _i9.Future<_i4.CreatePostResponse> createPost({ + required String? community, + String? title, + String? content, + List<_i2.RichTextFacet>? facets, + _i4.ExternalEmbedInput? embed, + List? langs, + _i4.SelfLabels? labels, + }) => + (super.noSuchMethod( + Invocation.method(#createPost, [], { + #community: community, + #title: title, + #content: content, + #facets: facets, + #embed: embed, + #langs: langs, + #labels: labels, + }), + returnValue: _i9.Future<_i4.CreatePostResponse>.value( + _FakeCreatePostResponse_4( + this, + Invocation.method(#createPost, [], { + #community: community, + #title: title, + #content: content, + #facets: facets, + #embed: embed, + #langs: langs, + #labels: labels, + }), + ), + ), + ) + as _i9.Future<_i4.CreatePostResponse>); + + @override + _i9.Future deletePost({required String? uri}) => + (super.noSuchMethod( + Invocation.method(#deletePost, [], {#uri: uri}), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future<_i4.CreateCommunityResponse> createCommunity({ + required String? name, + required String? displayName, + required String? description, + String? visibility = 'public', + }) => + (super.noSuchMethod( + Invocation.method(#createCommunity, [], { + #name: name, + #displayName: displayName, + #description: description, + #visibility: visibility, + }), + returnValue: _i9.Future<_i4.CreateCommunityResponse>.value( + _FakeCreateCommunityResponse_5( + this, + Invocation.method(#createCommunity, [], { + #name: name, + #displayName: displayName, + #description: description, + #visibility: visibility, + }), + ), + ), + ) + as _i9.Future<_i4.CreateCommunityResponse>); + + @override + _i9.Future<_i5.UserProfile> getProfile({required String? actor}) => + (super.noSuchMethod( + Invocation.method(#getProfile, [], {#actor: actor}), + returnValue: _i9.Future<_i5.UserProfile>.value( + _FakeUserProfile_6( + this, + Invocation.method(#getProfile, [], {#actor: actor}), + ), + ), + ) + as _i9.Future<_i5.UserProfile>); + + @override + _i9.Future<_i2.TimelineResponse> getAuthorPosts({ + required String? actor, + String? filter, + String? community, + int? limit = 15, + String? cursor, + }) => + (super.noSuchMethod( + Invocation.method(#getAuthorPosts, [], { + #actor: actor, + #filter: filter, + #community: community, + #limit: limit, + #cursor: cursor, + }), + returnValue: _i9.Future<_i2.TimelineResponse>.value( + _FakeTimelineResponse_0( + this, + Invocation.method(#getAuthorPosts, [], { + #actor: actor, + #filter: filter, + #community: community, + #limit: limit, + #cursor: cursor, + }), + ), + ), + ) + as _i9.Future<_i2.TimelineResponse>); + + @override + _i9.Future<_i3.ActorCommentsResponse> getActorComments({ + required String? actor, + String? community, + int? limit = 50, + String? cursor, + }) => + (super.noSuchMethod( + Invocation.method(#getActorComments, [], { + #actor: actor, + #community: community, + #limit: limit, + #cursor: cursor, + }), + returnValue: _i9.Future<_i3.ActorCommentsResponse>.value( + _FakeActorCommentsResponse_7( + this, + Invocation.method(#getActorComments, [], { + #actor: actor, + #community: community, + #limit: limit, + #cursor: cursor, + }), + ), + ), + ) + as _i9.Future<_i3.ActorCommentsResponse>); + + @override + _i9.Future subscribeToCommunity({required String? community}) => + (super.noSuchMethod( + Invocation.method(#subscribeToCommunity, [], { + #community: community, + }), + returnValue: _i9.Future.value( + _i12.dummyValue( + this, + Invocation.method(#subscribeToCommunity, [], { + #community: community, + }), + ), + ), + ) + as _i9.Future); + + @override + _i9.Future unsubscribeFromCommunity({required String? community}) => + (super.noSuchMethod( + Invocation.method(#unsubscribeFromCommunity, [], { + #community: community, + }), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future blockUser({required String? actor}) => + (super.noSuchMethod( + Invocation.method(#blockUser, [], {#actor: actor}), + returnValue: _i9.Future.value( + _i12.dummyValue( + this, + Invocation.method(#blockUser, [], {#actor: actor}), + ), + ), + ) + as _i9.Future); + + @override + _i9.Future unblockUser({required String? actor}) => + (super.noSuchMethod( + Invocation.method(#unblockUser, [], {#actor: actor}), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future blockCommunity({required String? community}) => + (super.noSuchMethod( + Invocation.method(#blockCommunity, [], {#community: community}), + returnValue: _i9.Future.value( + _i12.dummyValue( + this, + Invocation.method(#blockCommunity, [], {#community: community}), + ), + ), + ) + as _i9.Future); + + @override + _i9.Future unblockCommunity({required String? community}) => + (super.noSuchMethod( + Invocation.method(#unblockCommunity, [], {#community: community}), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); + + @override + _i9.Future<_i4.CreateCommunityResponse> updateCommunity({ + required String? communityDid, + required _i13.Uint8List? imageBytes, + required String? mimeType, + }) => + (super.noSuchMethod( + Invocation.method(#updateCommunity, [], { + #communityDid: communityDid, + #imageBytes: imageBytes, + #mimeType: mimeType, + }), + returnValue: _i9.Future<_i4.CreateCommunityResponse>.value( + _FakeCreateCommunityResponse_5( + this, + Invocation.method(#updateCommunity, [], { + #communityDid: communityDid, + #imageBytes: imageBytes, + #mimeType: mimeType, + }), + ), + ), + ) + as _i9.Future<_i4.CreateCommunityResponse>); + + @override + _i9.Future<_i6.UpdateProfileResponse> updateProfile({ + String? displayName, + String? bio, + _i13.Uint8List? avatarBytes, + String? avatarMimeType, + _i13.Uint8List? bannerBytes, + String? bannerMimeType, + }) => + (super.noSuchMethod( + Invocation.method(#updateProfile, [], { + #displayName: displayName, + #bio: bio, + #avatarBytes: avatarBytes, + #avatarMimeType: avatarMimeType, + #bannerBytes: bannerBytes, + #bannerMimeType: bannerMimeType, + }), + returnValue: _i9.Future<_i6.UpdateProfileResponse>.value( + _FakeUpdateProfileResponse_8( + this, + Invocation.method(#updateProfile, [], { + #displayName: displayName, + #bio: bio, + #avatarBytes: avatarBytes, + #avatarMimeType: avatarMimeType, + #bannerBytes: bannerBytes, + #bannerMimeType: bannerMimeType, + }), + ), + ), + ) + as _i9.Future<_i6.UpdateProfileResponse>); + + @override + _i9.Future submitReport({ + required String? targetUri, + required String? reason, + String? explanation, + }) => + (super.noSuchMethod( + Invocation.method(#submitReport, [], { + #targetUri: targetUri, + #reason: reason, + #explanation: explanation, + }), + returnValue: _i9.Future.value(0), + ) + as _i9.Future); + + @override + void dispose() => super.noSuchMethod( + Invocation.method(#dispose, []), + returnValueForMissingStub: null, + ); +} + +/// A class which mocks [VoteProvider]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockVoteProvider extends _i1.Mock implements _i14.VoteProvider { + MockVoteProvider() { + _i1.throwOnMissingStub(this); + } + + @override + bool get hasListeners => + (super.noSuchMethod(Invocation.getter(#hasListeners), returnValue: false) + as bool); + + @override + void dispose() => super.noSuchMethod( + Invocation.method(#dispose, []), + returnValueForMissingStub: null, + ); + + @override + _i14.VoteState? getVoteState(String? postUri) => + (super.noSuchMethod(Invocation.method(#getVoteState, [postUri])) + as _i14.VoteState?); + + @override + bool isLiked(String? postUri) => + (super.noSuchMethod( + Invocation.method(#isLiked, [postUri]), + returnValue: false, + ) + as bool); + + @override + bool isPending(String? postUri) => + (super.noSuchMethod( + Invocation.method(#isPending, [postUri]), + returnValue: false, + ) + as bool); + + @override + int getAdjustedScore(String? postUri, int? serverScore) => + (super.noSuchMethod( + Invocation.method(#getAdjustedScore, [postUri, serverScore]), + returnValue: 0, + ) + as int); + + @override + _i9.Future toggleVote({ + required String? postUri, + required String? postCid, + String? direction = 'up', + }) => + (super.noSuchMethod( + Invocation.method(#toggleVote, [], { + #postUri: postUri, + #postCid: postCid, + #direction: direction, + }), + returnValue: _i9.Future.value(false), + ) + as _i9.Future); + + @override + void setInitialVoteState({ + required String? postUri, + String? voteDirection, + String? voteUri, + }) => super.noSuchMethod( + Invocation.method(#setInitialVoteState, [], { + #postUri: postUri, + #voteDirection: voteDirection, + #voteUri: voteUri, + }), + returnValueForMissingStub: null, + ); + + @override + void clear() => super.noSuchMethod( + Invocation.method(#clear, []), + returnValueForMissingStub: null, + ); + + @override + void addListener(_i10.VoidCallback? listener) => super.noSuchMethod( + Invocation.method(#addListener, [listener]), + returnValueForMissingStub: null, + ); + + @override + void removeListener(_i10.VoidCallback? listener) => super.noSuchMethod( + Invocation.method(#removeListener, [listener]), + returnValueForMissingStub: null, + ); + + @override + void notifyListeners() => super.noSuchMethod( + Invocation.method(#notifyListeners, []), + returnValueForMissingStub: null, + ); +} + +/// A class which mocks [CommentService]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockCommentService extends _i1.Mock implements _i7.CommentService { + MockCommentService() { + _i1.throwOnMissingStub(this); + } + + @override + _i9.Future<_i7.CreateCommentResponse> createComment({ + required String? rootUri, + required String? rootCid, + required String? parentUri, + required String? parentCid, + required String? content, + List<_i2.RichTextFacet>? contentFacets, + }) => + (super.noSuchMethod( + Invocation.method(#createComment, [], { + #rootUri: rootUri, + #rootCid: rootCid, + #parentUri: parentUri, + #parentCid: parentCid, + #content: content, + #contentFacets: contentFacets, + }), + returnValue: _i9.Future<_i7.CreateCommentResponse>.value( + _FakeCreateCommentResponse_9( + this, + Invocation.method(#createComment, [], { + #rootUri: rootUri, + #rootCid: rootCid, + #parentUri: parentUri, + #parentCid: parentCid, + #content: content, + #contentFacets: contentFacets, + }), + ), + ), + ) + as _i9.Future<_i7.CreateCommentResponse>); + + @override + _i9.Future deleteComment({required String? uri}) => + (super.noSuchMethod( + Invocation.method(#deleteComment, [], {#uri: uri}), + returnValue: _i9.Future.value(), + returnValueForMissingStub: _i9.Future.value(), + ) + as _i9.Future); +} diff --git a/test/widget_test.dart b/test/widget_test.dart index e866d0f..5dabcb9 100644 --- a/test/widget_test.dart +++ b/test/widget_test.dart @@ -1,20 +1,52 @@ import 'package:coves_flutter/main.dart'; import 'package:coves_flutter/providers/auth_provider.dart'; +import 'package:coves_flutter/providers/community_guidelines_provider.dart'; +import 'package:coves_flutter/providers/eula_provider.dart'; import 'package:coves_flutter/providers/multi_feed_provider.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:provider/provider.dart'; +// Fakes mirror test/router/post_route_test.dart: gates report accepted so the +// router doesn't redirect to /eula before MaterialApp settles. +class FakeAuthProvider extends AuthProvider { + @override + bool get isAuthenticated => false; + + @override + bool get isLoading => false; +} + +class FakeEulaProvider extends EulaProvider { + @override + bool get hasAccepted => true; + + @override + bool get isLoading => false; +} + +class FakeGuidelinesProvider extends CommunityGuidelinesProvider { + @override + bool get hasAccepted => true; + + @override + bool get isLoading => false; +} + void main() { testWidgets('CovesApp smoke test', (WidgetTester tester) async { - // Create auth provider - final authProvider = AuthProvider(); + final authProvider = FakeAuthProvider(); - // Build our app and trigger a frame. await tester.pumpWidget( MultiProvider( providers: [ - ChangeNotifierProvider.value(value: authProvider), + ChangeNotifierProvider.value(value: authProvider), + ChangeNotifierProvider( + create: (_) => FakeEulaProvider(), + ), + ChangeNotifierProvider( + create: (_) => FakeGuidelinesProvider(), + ), ChangeNotifierProvider( create: (_) => MultiFeedProvider(authProvider), ), @@ -23,10 +55,8 @@ void main() { ), ); - // Allow the router to initialize await tester.pumpAndSettle(); - // Verify that the app builds without crashing expect(find.byType(MaterialApp), findsOneWidget); }); } diff --git a/test/widgets/comment_card_test.dart b/test/widgets/comment_card_test.dart index c0e3a6c..dd631ea 100644 --- a/test/widgets/comment_card_test.dart +++ b/test/widgets/comment_card_test.dart @@ -64,12 +64,18 @@ void main() { expect(emptyComment.content, ''); expect(emptyComment.record, isNotNull); }); + + test('isTombstoned is set for deleted comments only', () { + // CommentCard renders the tombstone placeholder off this getter. + // (The author == null arm is defensive: CommentView asserts that + // non-deleted comments always carry an author.) + expect(createComment(isDeleted: true).isTombstoned, isTrue); + expect(createComment().isTombstoned, isFalse); + }); }); - // Widget tests are skipped due to Provider type compatibility issues. - // See comment_thread_test.dart for similar pattern. - // The deleted comment UI is verified through: - // 1. Model tests above confirming data structure - // 2. Manual testing - // 3. The CommentCard code that checks isDeleted before rendering + // CommentCard rendering (including the tombstone placeholder for deleted + // comments) is covered by the widget tests in comment_thread_test.dart, + // which drive CommentCard through real providers over shared generated + // mocks (test/test_helpers/test_mocks.dart). } diff --git a/test/widgets/comment_thread_test.dart b/test/widgets/comment_thread_test.dart index 91f4c43..bd30b54 100644 --- a/test/widgets/comment_thread_test.dart +++ b/test/widgets/comment_thread_test.dart @@ -9,10 +9,10 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:provider/provider.dart'; -// Reuse the generated mockito mocks (real provider types) from the -// comments provider tests so Consumer/Consumer -// lookups inside CommentCard resolve correctly. -import '../providers/comments_provider_test.mocks.dart'; +// Shared generated mockito mocks (real provider types) so +// Consumer/Consumer lookups inside CommentCard +// resolve correctly. +import '../test_helpers/test_mocks.dart'; // NOTE: CommentThread.countDescendants was removed in e134a88 — the widget // now uses the API-provided `stats.replyCount` for collapsed/continue-thread @@ -80,6 +80,8 @@ void main() { bool isDeleted = false, String? deletionReason, List? replies, + bool hasMore = false, + String? repliesCursor, }) { return ThreadViewComment( comment: createComment( @@ -90,6 +92,8 @@ void main() { deletionReason: deletionReason, ), replies: replies, + hasMore: hasMore, + repliesCursor: repliesCursor, ); } @@ -100,6 +104,8 @@ void main() { void Function(ThreadViewComment)? onCommentTap, void Function(String uri)? onCollapseToggle, void Function(ThreadViewComment, List)? onContinueThread, + void Function(ThreadViewComment)? onLoadMoreReplies, + Set loadingMoreReplies = const {}, Set collapsedComments = const {}, List ancestors = const [], }) { @@ -119,6 +125,8 @@ void main() { onCommentTap: onCommentTap, onCollapseToggle: onCollapseToggle, onContinueThread: onContinueThread, + onLoadMoreReplies: onLoadMoreReplies, + loadingMoreReplies: loadingMoreReplies, collapsedComments: collapsedComments, ancestors: ancestors, ), @@ -304,4 +312,76 @@ void main() { expect(find.text('Surviving reply'), findsOneWidget); }); }); + + group('Load more replies button', () { + testWidgets('renders when the thread has more replies', (tester) async { + final thread = createThread( + uri: 'comment/1', + content: 'Parent', + hasMore: true, + ); + + await tester.pumpWidget(createTestWidget(thread)); + + expect(find.text('Load more replies'), findsOneWidget); + expect(find.byIcon(Icons.add_circle_outline), findsOneWidget); + expect(find.byType(CircularProgressIndicator), findsNothing); + }); + + testWidgets('does not render without more replies', (tester) async { + final thread = createThread(uri: 'comment/1', content: 'Parent'); + + await tester.pumpWidget(createTestWidget(thread)); + + expect(find.text('Load more replies'), findsNothing); + }); + + testWidgets('tap invokes onLoadMoreReplies with the thread', + (tester) async { + ThreadViewComment? tapped; + final thread = createThread( + uri: 'comment/1', + content: 'Parent', + hasMore: true, + ); + + await tester.pumpWidget(createTestWidget( + thread, + onLoadMoreReplies: (t) => tapped = t, + )); + + await tester.tap(find.text('Load more replies')); + await tester.pump(); + + expect(tapped, isNotNull); + expect(tapped!.comment.uri, 'comment/1'); + }); + + testWidgets( + 'in-flight fetch shows spinner, loading label, and disables tap', + (tester) async { + var tapCount = 0; + final thread = createThread( + uri: 'comment/1', + content: 'Parent', + hasMore: true, + ); + + await tester.pumpWidget(createTestWidget( + thread, + onLoadMoreReplies: (_) => tapCount++, + loadingMoreReplies: {'comment/1'}, + )); + + expect(find.text('Loading replies…'), findsOneWidget); + expect(find.text('Load more replies'), findsNothing); + expect(find.byType(CircularProgressIndicator), findsOneWidget); + expect(find.byIcon(Icons.add_circle_outline), findsNothing); + + // Tap is disabled while loading + await tester.tap(find.text('Loading replies…'), warnIfMissed: false); + await tester.pump(); + expect(tapCount, 0); + }); + }); } diff --git a/test/widgets/focused_thread_screen_test.dart b/test/widgets/focused_thread_screen_test.dart index 4049a2f..f511d0e 100644 --- a/test/widgets/focused_thread_screen_test.dart +++ b/test/widgets/focused_thread_screen_test.dart @@ -1,217 +1,566 @@ import 'package:coves_flutter/models/comment.dart'; import 'package:coves_flutter/models/post.dart'; +import 'package:coves_flutter/providers/auth_provider.dart'; +import 'package:coves_flutter/providers/block_provider.dart'; import 'package:coves_flutter/providers/comments_provider.dart'; +import 'package:coves_flutter/providers/vote_provider.dart'; import 'package:coves_flutter/screens/home/focused_thread_screen.dart'; +import 'package:coves_flutter/services/api_exceptions.dart'; +import 'package:coves_flutter/widgets/comment_card.dart'; +import 'package:coves_flutter/widgets/loading_error_states.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; import 'package:provider/provider.dart'; -import '../test_helpers/mock_providers.dart'; +// Shared generated mockito mocks (real provider types) so provider lookups +// inside the screen and CommentCard resolve correctly. +import '../test_helpers/test_mocks.dart'; void main() { + const postUri = 'at://did:plc:test/social.coves.community.post/123'; + const postCid = 'post-cid'; + const authorDid = 'did:plc:author'; + late MockAuthProvider mockAuthProvider; late MockVoteProvider mockVoteProvider; - late MockCommentsProvider mockCommentsProvider; + late MockCovesApiService mockApiService; + late MockCommentService mockCommentService; + late BlockProvider blockProvider; + late CommentsProvider commentsProvider; setUp(() { mockAuthProvider = MockAuthProvider(); mockVoteProvider = MockVoteProvider(); - mockCommentsProvider = MockCommentsProvider( - postUri: 'at://did:plc:test/post/123', - postCid: 'post-cid', + mockApiService = MockCovesApiService(); + mockCommentService = MockCommentService(); + blockProvider = BlockProvider( + apiService: mockApiService, + authProvider: mockAuthProvider, + ); + + // Signed-out by default: CommentCard hides the actions menu and the + // vote button renders in the un-liked state. + when(mockAuthProvider.isAuthenticated).thenReturn(false); + when(mockVoteProvider.isLiked(any)).thenReturn(false); + when(mockVoteProvider.getAdjustedScore(any, any)).thenAnswer( + (invocation) => invocation.positionalArguments[1] as int, + ); + + commentsProvider = CommentsProvider( + mockAuthProvider, + postUri: postUri, + postCid: postCid, + apiService: mockApiService, + voteProvider: mockVoteProvider, + commentService: mockCommentService, ); }); tearDown(() { - mockCommentsProvider.dispose(); + commentsProvider.dispose(); }); - /// Helper to create a test comment + /// Stubs every getComments call. [responder] receives the parentRkey of + /// the request (null for a top-level thread fetch) and returns the + /// response to serve. + void stubGetComments( + CommentsResponse Function(String? parentRkey) responder, + ) { + when(mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + parentRkey: anyNamed('parentRkey'), + )).thenAnswer( + (invocation) async => + responder(invocation.namedArguments[#parentRkey] as String?), + ); + } + + /// Helper to create a test comment. The rkey (last URI segment) is what + /// loadMoreReplies sends as parentRkey. CommentView createComment({ - required String uri, + required String rkey, String content = 'Test comment', String handle = 'test.user', + int replyCount = 0, }) { + final uri = 'at://did:plc:test/social.coves.community.comment/$rkey'; return CommentView( uri: uri, - cid: 'cid-$uri', + cid: 'cid-$rkey', record: CommentRecord(content: content), createdAt: DateTime(2025), indexedAt: DateTime(2025), - author: AuthorView(did: 'did:plc:author', handle: handle), - post: CommentRef(uri: 'at://did:plc:test/post/123', cid: 'post-cid'), - stats: CommentStats(upvotes: 5, downvotes: 1, score: 4), + author: AuthorView(did: authorDid, handle: handle), + post: CommentRef(uri: postUri, cid: postCid), + stats: CommentStats( + upvotes: 5, + downvotes: 1, + score: 4, + replyCount: replyCount, + ), ); } /// Helper to create a thread with nested replies ThreadViewComment createThread({ - required String uri, + required String rkey, String content = 'Test comment', + int replyCount = 0, List? replies, + bool hasMore = false, }) { return ThreadViewComment( - comment: createComment(uri: uri, content: content), + comment: createComment( + rkey: rkey, + content: content, + replyCount: replyCount, + ), replies: replies, + hasMore: hasMore, ); } + CommentsResponse response( + List comments, { + String? cursor, + }) { + return CommentsResponse(post: null, cursor: cursor, comments: comments); + } + Widget createTestWidget({ required ThreadViewComment thread, List ancestors = const [], - Future Function(String, List, ThreadViewComment)? onReply, + Future Function(String, List, ThreadViewComment)? + onReply, }) { return MultiProvider( providers: [ - ChangeNotifierProvider.value(value: mockAuthProvider), - ChangeNotifierProvider.value(value: mockVoteProvider), + ChangeNotifierProvider.value(value: mockAuthProvider), + ChangeNotifierProvider.value(value: mockVoteProvider), + ChangeNotifierProvider.value(value: blockProvider), ], child: MaterialApp( home: FocusedThreadScreen( thread: thread, ancestors: ancestors, onReply: onReply ?? (content, facets, parent) async {}, - // Note: Using mock cast - tests are skipped so this won't actually run - commentsProvider: mockCommentsProvider as CommentsProvider, + commentsProvider: commentsProvider, ), ), ); } - group( - 'FocusedThreadScreen', - skip: 'Provider type compatibility issues - needs mock refactoring', - () { - testWidgets('renders anchor comment', (tester) async { - final thread = createThread( - uri: 'comment/anchor', - content: 'This is the anchor comment', - ); + /// The screen auto-scrolls the anchor to the top on entry, which hides + /// the floating app bar and pushes ancestors out of the sliver viewport. + /// Scroll back to the top so those widgets are built again. + Future scrollBackToTop(WidgetTester tester) async { + await tester.drag(find.byType(CustomScrollView), const Offset(0, 800)); + await tester.pumpAndSettle(); + } - await tester.pumpWidget(createTestWidget(thread: thread)); - await tester.pumpAndSettle(); + group('FocusedThreadScreen rendering', () { + setUp(() { + // Entry hydration resolves to an empty subtree page: the screen keeps + // rendering the snapshot it was given. + stubGetComments((_) => response([])); + }); - expect(find.text('This is the anchor comment'), findsOneWidget); - }); + testWidgets('renders anchor comment', (tester) async { + final thread = createThread( + rkey: 'anchor', + content: 'This is the anchor comment', + ); - testWidgets('renders ancestor comments', (tester) async { - final ancestor1 = createThread( - uri: 'comment/1', - content: 'First ancestor', - ); - final ancestor2 = createThread( - uri: 'comment/2', - content: 'Second ancestor', - ); - final anchor = createThread( - uri: 'comment/anchor', - content: 'Anchor comment', - ); - - await tester.pumpWidget(createTestWidget( - thread: anchor, - ancestors: [ancestor1, ancestor2], - )); - await tester.pumpAndSettle(); - - expect(find.text('First ancestor'), findsOneWidget); - expect(find.text('Second ancestor'), findsOneWidget); - expect(find.text('Anchor comment'), findsOneWidget); - }); + await tester.pumpWidget(createTestWidget(thread: thread)); + await tester.pumpAndSettle(); + + expect(find.text('This is the anchor comment'), findsOneWidget); + }); + + testWidgets('renders ancestor comments', (tester) async { + final ancestor1 = createThread(rkey: 'a1', content: 'First ancestor'); + final ancestor2 = createThread(rkey: 'a2', content: 'Second ancestor'); + final anchor = createThread(rkey: 'anchor', content: 'Anchor comment'); + + await tester.pumpWidget(createTestWidget( + thread: anchor, + ancestors: [ancestor1, ancestor2], + )); + await tester.pumpAndSettle(); + await scrollBackToTop(tester); + + expect(find.text('First ancestor'), findsOneWidget); + expect(find.text('Second ancestor'), findsOneWidget); + expect(find.text('Anchor comment'), findsOneWidget); + }); + + testWidgets('renders replies below anchor', (tester) async { + final thread = createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [ + createThread(rkey: 'r1', content: 'First reply'), + createThread(rkey: 'r2', content: 'Second reply'), + ], + ); + + await tester.pumpWidget(createTestWidget(thread: thread)); + await tester.pumpAndSettle(); + + expect(find.text('Anchor comment'), findsOneWidget); + expect(find.text('First reply'), findsOneWidget); + expect(find.text('Second reply'), findsOneWidget); + }); + + testWidgets('shows empty state when no replies', (tester) async { + final thread = createThread( + rkey: 'anchor', + content: 'Anchor with no replies', + ); + + await tester.pumpWidget(createTestWidget(thread: thread)); + await tester.pumpAndSettle(); + + expect(find.text('No replies yet'), findsOneWidget); + expect( + find.text('Be the first to reply to this comment'), + findsOneWidget, + ); + }); + + testWidgets('does not duplicate thread in ancestors', (tester) async { + final ancestor = createThread(rkey: 'a1', content: 'Ancestor content'); + final anchor = createThread(rkey: 'anchor', content: 'Anchor content'); + + await tester.pumpWidget(createTestWidget( + thread: anchor, + ancestors: [ancestor], + )); + await tester.pumpAndSettle(); + await scrollBackToTop(tester); + + expect(find.text('Anchor content'), findsOneWidget); + expect(find.text('Ancestor content'), findsOneWidget); + }); + + testWidgets('shows Thread title in app bar', (tester) async { + final thread = createThread(rkey: 'anchor'); + + await tester.pumpWidget(createTestWidget(thread: thread)); + await tester.pumpAndSettle(); + await scrollBackToTop(tester); + + expect(find.text('Thread'), findsOneWidget); + }); + + testWidgets('ancestors are styled with reduced opacity', (tester) async { + final ancestor = createThread(rkey: 'a1', content: 'Ancestor'); + final anchor = createThread(rkey: 'anchor', content: 'Anchor'); + + await tester.pumpWidget(createTestWidget( + thread: anchor, + ancestors: [ancestor], + )); + await tester.pumpAndSettle(); + await scrollBackToTop(tester); + + final opacityFinder = find.ancestor( + of: find.text('Ancestor'), + matching: find.byType(Opacity), + ); - testWidgets('renders replies below anchor', (tester) async { - final thread = createThread( - uri: 'comment/anchor', - content: 'Anchor comment', - replies: [ - createThread(uri: 'comment/reply1', content: 'First reply'), - createThread(uri: 'comment/reply2', content: 'Second reply'), - ], - ); - - await tester.pumpWidget(createTestWidget(thread: thread)); - await tester.pumpAndSettle(); - - expect(find.text('Anchor comment'), findsOneWidget); - expect(find.text('First reply'), findsOneWidget); - expect(find.text('Second reply'), findsOneWidget); + expect(opacityFinder, findsOneWidget); + + final opacity = tester.widget(opacityFinder); + expect(opacity.opacity, 0.6); + }); + }); + + group('FocusedThreadScreen hydration', () { + testWidgets('hydrates the anchor subtree on entry (deep replies render)', + (tester) async { + // Snapshot truncated by the original fetch depth: only one shallow + // reply. The server has a deeper tree behind it. + final snapshot = createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [createThread(rkey: 'r1', content: 'Shallow reply')], + ); + + stubGetComments((parentRkey) { + expect(parentRkey, 'anchor'); + return response([ + createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [ + createThread( + rkey: 'r1', + content: 'Shallow reply', + replies: [ + createThread(rkey: 'r1a', content: 'Deep hydrated reply'), + ], + ), + ], + ), + ]); }); - testWidgets('shows empty state when no replies', (tester) async { - final thread = createThread( - uri: 'comment/anchor', - content: 'Anchor with no replies', - ); + await tester.pumpWidget(createTestWidget(thread: snapshot)); + await tester.pumpAndSettle(); + + expect(find.text('Shallow reply'), findsOneWidget); + expect(find.text('Deep hydrated reply'), findsOneWidget); + }); + + testWidgets('hydration failure keeps the snapshot visible (non-fatal)', + (tester) async { + final snapshot = createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [createThread(rkey: 'r1', content: 'Existing reply')], + ); + + stubGetComments((_) => throw ApiException('boom')); + + await tester.pumpWidget(createTestWidget(thread: snapshot)); + await tester.pumpAndSettle(); + + // Snapshot still renders, and no error/empty state is shown + expect(find.text('Anchor comment'), findsOneWidget); + expect(find.text('Existing reply'), findsOneWidget); + expect(find.byType(InlineError), findsNothing); + expect(find.text('No replies yet'), findsNothing); + }); - await tester.pumpWidget(createTestWidget(thread: thread)); - await tester.pumpAndSettle(); + testWidgets( + 'hydration failure with an empty snapshot shows a retryable error, ' + 'and retry recovers', (tester) async { + final snapshot = createThread(rkey: 'anchor', content: 'Anchor comment'); - expect(find.text('No replies yet'), findsOneWidget); - expect( - find.text('Be the first to reply to this comment'), - findsOneWidget, - ); + var shouldFail = true; + stubGetComments((_) { + if (shouldFail) { + throw ApiException('boom'); + } + return response([ + createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [createThread(rkey: 'r1', content: 'Recovered reply')], + ), + ]); }); - testWidgets('does not duplicate thread in ancestors', (tester) async { - // This tests the fix for the duplication bug - final ancestor = createThread( - uri: 'comment/ancestor', - content: 'Ancestor content', - ); - final anchor = createThread( - uri: 'comment/anchor', - content: 'Anchor content', - ); - - await tester.pumpWidget(createTestWidget( - thread: anchor, - ancestors: [ancestor], - )); - await tester.pumpAndSettle(); - - // Anchor should appear exactly once - expect(find.text('Anchor content'), findsOneWidget); - // Ancestor should appear exactly once - expect(find.text('Ancestor content'), findsOneWidget); + await tester.pumpWidget(createTestWidget(thread: snapshot)); + await tester.pumpAndSettle(); + + // Unknown state must not claim "No replies yet" - show retryable error + expect(find.byType(InlineError), findsOneWidget); + expect(find.text('No replies yet'), findsNothing); + + shouldFail = false; + await tester.tap(find.text('Retry')); + await tester.pumpAndSettle(); + + expect(find.byType(InlineError), findsNothing); + expect(find.text('Recovered reply'), findsOneWidget); + }); + }); + + group('FocusedThreadScreen load more', () { + testWidgets('nested load-more renders newly fetched grandchild', + (tester) async { + final snapshot = createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [createThread(rkey: 'r1', content: 'Child reply')], + ); + + stubGetComments((parentRkey) { + if (parentRkey == 'anchor') { + // Hydration: child has more replies behind the sibling cap + return response([ + createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [ + createThread( + rkey: 'r1', + content: 'Child reply', + replyCount: 1, + hasMore: true, + ), + ], + ), + ]); + } + expect(parentRkey, 'r1'); + return response([ + createThread( + rkey: 'r1', + content: 'Child reply', + replies: [ + createThread(rkey: 'r1a', content: 'Grandchild reply'), + ], + ), + ]); }); - testWidgets('shows Thread title in app bar', (tester) async { - final thread = createThread(uri: 'comment/1'); + await tester.pumpWidget(createTestWidget(thread: snapshot)); + await tester.pumpAndSettle(); + + expect(find.text('Grandchild reply'), findsNothing); + expect(find.text('Load more replies'), findsOneWidget); + + await tester.tap(find.text('Load more replies')); + await tester.pumpAndSettle(); + + expect(find.text('Grandchild reply'), findsOneWidget); + }); - await tester.pumpWidget(createTestWidget(thread: thread)); - await tester.pumpAndSettle(); + testWidgets( + 'anchor with more direct replies shows a load-more affordance ' + 'that fetches the next page', (tester) async { + final snapshot = createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [createThread(rkey: 'r1', content: 'First page reply')], + ); - expect(find.text('Thread'), findsOneWidget); + var anchorFetches = 0; + stubGetComments((parentRkey) { + expect(parentRkey, 'anchor'); + anchorFetches++; + if (anchorFetches == 1) { + // Hydration: first page of the anchor's direct replies + return response( + [ + createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [ + createThread(rkey: 'r1', content: 'First page reply'), + ], + ), + ], + cursor: 'page-2', + ); + } + return response([ + createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [ + createThread(rkey: 'r1', content: 'First page reply'), + createThread(rkey: 'r2', content: 'Second page reply'), + ], + ), + ]); }); - testWidgets('ancestors are styled with reduced opacity', (tester) async { - final ancestor = createThread( - uri: 'comment/ancestor', - content: 'Ancestor', - ); - final anchor = createThread( - uri: 'comment/anchor', - content: 'Anchor', - ); - - await tester.pumpWidget(createTestWidget( - thread: anchor, - ancestors: [ancestor], - )); - await tester.pumpAndSettle(); - - // Find the Opacity widget wrapping ancestor - final opacityFinder = find.ancestor( - of: find.text('Ancestor'), - matching: find.byType(Opacity), - ); - - expect(opacityFinder, findsOneWidget); - - final opacity = tester.widget(opacityFinder); - expect(opacity.opacity, 0.6); + await tester.pumpWidget(createTestWidget(thread: snapshot)); + await tester.pumpAndSettle(); + + // The anchor's own hasMore renders the affordance at the anchor level + expect(find.text('Load more replies'), findsOneWidget); + expect(find.text('Second page reply'), findsNothing); + + await tester.tap(find.text('Load more replies')); + await tester.pumpAndSettle(); + + expect(anchorFetches, 2); + expect(find.text('First page reply'), findsOneWidget); + expect(find.text('Second page reply'), findsOneWidget); + expect(find.text('Load more replies'), findsNothing); + }); + }); + + group('FocusedThreadScreen delete', () { + testWidgets('deleting a reply refetches the subtree and removes it', + (tester) async { + // The delete flow awaits HapticFeedback; without a handler the + // platform channel raises MissingPluginException in tests. + tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + SystemChannels.platform, + (methodCall) async => null, + ); + + // Signed in as the reply's author so the delete menu item shows + when(mockAuthProvider.isAuthenticated).thenReturn(true); + when(mockAuthProvider.did).thenReturn(authorDid); + + var deleted = false; + when(mockCommentService.deleteComment(uri: anyNamed('uri'))) + .thenAnswer((_) async => deleted = true); + + stubGetComments((parentRkey) { + if (parentRkey == null) { + // Top-level refresh triggered by deleteComment + return response([]); + } + expect(parentRkey, 'anchor'); + return response([ + createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [ + createThread(rkey: 'keep', content: 'Reply to keep'), + if (!deleted) + createThread(rkey: 'gone', content: 'Reply to remove'), + ], + ), + ]); }); - }, - ); + + final snapshot = createThread( + rkey: 'anchor', + content: 'Anchor comment', + replies: [ + createThread(rkey: 'keep', content: 'Reply to keep'), + createThread(rkey: 'gone', content: 'Reply to remove'), + ], + ); + + await tester.pumpWidget(createTestWidget(thread: snapshot)); + await tester.pumpAndSettle(); + + expect(find.text('Reply to remove'), findsOneWidget); + + // Open the actions menu on the reply's card and delete it + final replyCard = find + .ancestor( + of: find.text('Reply to remove'), + matching: find.byType(CommentCard), + ) + .first; + await tester.tap( + find.descendant(of: replyCard, matching: find.byIcon(Icons.more_horiz)), + ); + await tester.pumpAndSettle(); + + expect(find.text('Delete comment'), findsOneWidget); + await tester.tap(find.text('Delete comment')); + await tester.pumpAndSettle(); + + // Confirm in the dialog + expect(find.text('Delete Comment'), findsOneWidget); + await tester.tap(find.text('Delete')); + await tester.pumpAndSettle(); + + // Dialog dismissed after confirming + expect(find.text('Delete Comment'), findsNothing); + + verify(mockCommentService.deleteComment(uri: anyNamed('uri'))).called(1); + expect(find.text('Reply to remove'), findsNothing); + expect(find.text('Reply to keep'), findsOneWidget); + }); + }); } diff --git a/test/widgets/post_detail_loader_test.dart b/test/widgets/post_detail_loader_test.dart index c9b5524..29bd8bf 100644 --- a/test/widgets/post_detail_loader_test.dart +++ b/test/widgets/post_detail_loader_test.dart @@ -34,6 +34,21 @@ class FakeAuthProvider extends AuthProvider { void main() { const testUri = 'at://did:plc:test/social.coves.community.post/abc123'; + /// Asserts a full-screen terminal state renders [title] in its app bar + /// and once more in the body content (NotFoundError shows it in both). + void expectScreenTitle(String title) { + expect( + find.descendant(of: find.byType(AppBar), matching: find.text(title)), + findsOneWidget, + reason: 'app bar should show "$title"', + ); + expect( + find.descendant(of: find.byType(Center), matching: find.text(title)), + findsOneWidget, + reason: 'body should show "$title"', + ); + } + /// Pumps the loader with an injectable fetcher. /// /// No providers are needed: the loader only touches AuthProvider when @@ -102,7 +117,7 @@ void main() { await tester.pumpAndSettle(); expect(find.byType(NotFoundError), findsOneWidget); - expect(find.text('Post Not Found'), findsNWidgets(2)); + expectScreenTitle('Post Not Found'); }); testWidgets('renders PostDetailScreen on successful fetch', (tester) async { @@ -164,7 +179,7 @@ void main() { ); await tester.pumpAndSettle(); - expect(find.text('Post Unavailable'), findsNWidgets(2)); + expectScreenTitle('Post Unavailable'); expect( find.text("This post is from an account you've blocked."), findsOneWidget, @@ -184,7 +199,7 @@ void main() { ); await tester.pumpAndSettle(); - expect(find.text('Post Unavailable'), findsNWidgets(2)); + expectScreenTitle('Post Unavailable'); expect(find.text('This post was removed by moderators.'), findsOneWidget); }); @@ -199,7 +214,7 @@ void main() { ); await tester.pumpAndSettle(); - expect(find.text('Post Unavailable'), findsNWidgets(2)); + expectScreenTitle('Post Unavailable'); expect( find.text("This post is unavailable because it's from a blocked " 'source.'), @@ -234,7 +249,7 @@ void main() { await tester.pumpAndSettle(); expect(fetchCount, 2); - expect(find.text('Post Not Found'), findsNWidgets(2)); + expectScreenTitle('Post Not Found'); }); testWidgets('5xx from the server shows error state with retry', ( @@ -283,7 +298,7 @@ void main() { ); await tester.pumpAndSettle(); - expect(find.text('Post Not Found'), findsNWidgets(2)); + expectScreenTitle('Post Not Found'); expect(find.byType(FullScreenError), findsNothing); }); @@ -302,7 +317,7 @@ void main() { await tester.pumpAndSettle(); expect(fetchCount, 0); - expect(find.text('Post Not Found'), findsNWidgets(2)); + expectScreenTitle('Post Not Found'); }); testWidgets('navigating to a new postUri refetches (didUpdateWidget)', ( @@ -350,7 +365,7 @@ void main() { await tester.pumpAndSettle(); expect(fetchedUris, [testUri, secondUri]); - expect(find.text('Post Not Found'), findsNWidgets(2)); + expectScreenTitle('Post Not Found'); // Stale first fetch completing late must NOT overwrite the newer result firstFetch.complete( @@ -358,7 +373,7 @@ void main() { ); await tester.pumpAndSettle(); - expect(find.text('Post Not Found'), findsNWidgets(2)); + expectScreenTitle('Post Not Found'); expect(find.text('Post Unavailable'), findsNothing); }); @@ -390,4 +405,74 @@ void main() { await tester.pumpWidget(const MaterialApp(home: Scaffold())); await tester.pumpAndSettle(); }); + + group('PostDetailScreen.displayedCommentCount', () { + // The server-side commentCount includes comments the viewer never sees + // (deleted, blocked, filtered). Only a fully resolved-but-empty thread + // may show the header's empty state. + test('shows 0 when the thread resolved empty with no more pages', () { + expect( + PostDetailScreen.displayedCommentCount( + serverCount: 7, + isLoading: false, + hasError: false, + hasComments: false, + hasMore: false, + ), + 0, + ); + }); + + test('keeps the server count while loading', () { + expect( + PostDetailScreen.displayedCommentCount( + serverCount: 7, + isLoading: true, + hasError: false, + hasComments: false, + hasMore: false, + ), + 7, + ); + }); + + test('keeps the server count when loading errored', () { + expect( + PostDetailScreen.displayedCommentCount( + serverCount: 7, + isLoading: false, + hasError: true, + hasComments: false, + hasMore: false, + ), + 7, + ); + }); + + test('keeps the server count while more pages may exist', () { + expect( + PostDetailScreen.displayedCommentCount( + serverCount: 7, + isLoading: false, + hasError: false, + hasComments: false, + hasMore: true, + ), + 7, + ); + }); + + test('keeps the server count when comments rendered', () { + expect( + PostDetailScreen.displayedCommentCount( + serverCount: 7, + isLoading: false, + hasError: false, + hasComments: true, + hasMore: false, + ), + 7, + ); + }); + }); } -- 2.51.2