diff --git a/lib/main.dart b/lib/main.dart index e3a2c74..dd96f59 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -32,6 +32,7 @@ import 'services/comment_service.dart'; import 'services/comments_provider_cache.dart'; import 'services/coves_api_service.dart'; import 'services/streamable_service.dart'; +import 'services/viewer_state_hydrator.dart'; import 'services/vote_service.dart'; import 'widgets/loading_error_states.dart'; @@ -176,6 +177,27 @@ Future bootstrapCovesApp() async { authProvider: authProvider, ), ), + // One hydrator for every fetch path that seeds viewer state (votes, + // community subscriptions) from a response. + // + // Registered AFTER VoteProvider and CommunitySubscriptionProvider + // because it reads both, and BEFORE the consumers below that read it + // in their `create`. Safe to capture the notifiers once: both are + // plain ChangeNotifierProvider(create:) instances, created once and + // never replaced, and every consumer proxy returns `previous ?? ...` + // so the `vote` and `subscription` arguments its `update` receives + // are discarded. (The `auth` argument is NOT discarded everywhere - + // UserProfileProvider's update forwards it to updateAuthProvider, + // which rebinds its hydrator.) + Provider( + create: + (context) => ViewerStateHydrator( + authProvider: authProvider, + voteProvider: context.read(), + subscriptionProvider: + context.read(), + ), + ), ChangeNotifierProxyProvider3< AuthProvider, VoteProvider, @@ -186,9 +208,7 @@ Future bootstrapCovesApp() async { (context) => MultiFeedProvider( authProvider, apiService: apiService, - voteProvider: context.read(), - subscriptionProvider: - context.read(), + hydrator: context.read(), ), update: (context, auth, vote, subscription, previous) { // Reuse existing provider to maintain state across rebuilds @@ -196,8 +216,7 @@ Future bootstrapCovesApp() async { MultiFeedProvider( auth, apiService: apiService, - voteProvider: vote, - subscriptionProvider: subscription, + hydrator: context.read(), ); }, ), @@ -210,6 +229,7 @@ Future bootstrapCovesApp() async { voteProvider: context.read(), commentService: commentService, apiService: apiService, + hydrator: context.read(), ), update: (context, auth, vote, previous) { // Reuse existing cache @@ -219,6 +239,7 @@ Future bootstrapCovesApp() async { voteProvider: vote, commentService: commentService, apiService: apiService, + hydrator: context.read(), ); }, dispose: (_, cache) => cache.dispose(), @@ -235,8 +256,12 @@ Future bootstrapCovesApp() async { (context) => UserProfileProvider( authProvider, apiService: apiService, - voteProvider: context.read(), commentService: commentService, + // Fully wired, subscriptions included: this surface calls + // hydrateFeedVotesOnly, so "profile posts never seed + // subscriptions" is a property of the call, not of a missing + // provider. + hydrator: context.read(), ), update: (context, auth, vote, previous) { // The shared apiService/commentService auth callbacks are bound @@ -253,8 +278,8 @@ Future bootstrapCovesApp() async { UserProfileProvider( auth, apiService: apiService, - voteProvider: vote, commentService: commentService, + hydrator: context.read(), ); }, ), diff --git a/lib/models/comment_thread_tree.dart b/lib/models/comment_thread_tree.dart new file mode 100644 index 0000000..5bbdf24 --- /dev/null +++ b/lib/models/comment_thread_tree.dart @@ -0,0 +1,229 @@ +import 'dart:collection'; + +import 'comment.dart'; + +/// The comment-thread algebra: lookup, replacement, subtree merging, and the +/// decision of what a load-more-replies response actually produces. +/// +/// A thin value wrapper over the `List` a thread is made +/// of. Pure by construction — no providers, no notifiers, no I/O, no +/// logging, no clock. Everything here is a function of its arguments, so it +/// needs no mocks to test directly, though today's coverage happens to +/// reach it through CommentsProvider's public surface instead. +/// +/// What deliberately stays outside: the fetch, staleness/generation guards, +/// the anchored-response contract check, the empty-response pagination +/// clear, `notifyListeners`, and any read or write of the provider's own +/// comment list. Those are orchestration; this is algebra. +/// +/// ## Reporting a missed replacement +/// +/// [replaceNode] returns a `replaced` flag alongside the new tree rather +/// than leaving callers to infer "nothing matched" from list identity. The +/// flag says one thing and says it precisely; identity said two things at +/// once — "the URI is not in this tree" AND "the replacement was already +/// the node sitting there" — and a caller could not tell them apart. That +/// ambiguity is why [nodes] can now be an unmodifiable view: nothing +/// depends on it being the same instance any more. +/// +/// ## Deliberate asymmetries +/// +/// [mergeSubtree] has branches that answer the same question in opposite +/// ways, and [subtreeFromResponse] picks between appending and merging on a +/// cursor whose value the caller captured at a different moment than the +/// node it is paired with. Each of those looks like a bug and is not; each +/// is documented at the branch that makes the choice, and each is pinned by +/// a characterization test. Do not harmonise them here — if the product +/// decision changes, change it together with its test. +class CommentThreadTree { + const CommentThreadTree(this._nodes); + + final List _nodes; + + /// The top-level comments of the thread, read-only. + /// + /// An unmodifiable view: this type calls itself pure, and handing out the + /// caller's growable list let any holder `add` straight into a provider's + /// live comment tree. Cheap — the view wraps, it does not copy. + List get nodes => UnmodifiableListView(_nodes); + + /// The node with [uri] anywhere in the tree, at any depth, or null. + ThreadViewComment? findByUri(String uri) { + for (final node in _nodes) { + final found = node.findByUri(uri); + if (found != null) { + return found; + } + } + return null; + } + + /// Whether a comment with [uri] exists anywhere in the tree. + bool containsUri(String uri) => findByUri(uri) != null; + + /// This tree with the node matching [replacement]'s URI replaced, plus + /// whether anything actually was. + /// + /// `replaced` is false when no node in the tree carries that URI — and + /// also, in principle, when [replacement] is the very instance already + /// sitting there, because [ThreadViewComment.replaceDescendant] returns + /// `this` on a match it does not have to change. Callers must not read + /// false as "the URI is absent"; it means "this tree is unchanged", which + /// is the only thing the walk below can honestly report. Test-pinned in + /// both directions. + /// + /// `tree` is `this` when nothing changed, so an unchanged result costs no + /// allocation. + ({CommentThreadTree tree, bool replaced}) replaceNode( + ThreadViewComment replacement, + ) { + var changed = false; + final mapped = []; + for (final node in _nodes) { + final result = node.replaceDescendant(replacement); + if (!identical(result, node)) { + changed = true; + } + mapped.add(result); + } + if (!changed) { + return (tree: this, replaced: false); + } + return (tree: CommentThreadTree(mapped), replaced: true); + } + + /// 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. + /// + /// The branches below disagree with each other on purpose. Every such + /// disagreement is tagged ASYMMETRY where it happens and is pinned by a + /// test; read the tags rather than trusting a count here. + static 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) { + // ASYMMETRY (deliberate, test-pinned): with nothing to preserve on + // either side the fresh node is returned VERBATIM, which drops BOTH + // existing.repliesCursor and existing.hasMore - the two fields the + // branch just below goes out of its way to keep. The only + // difference between the two cases is whether the existing branch + // had children. + return fresh; + } + // ASYMMETRY (deliberate, test-pinned): hasMore is taken from + // EXISTING here, and from FRESH on the recursive branch below. + 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). + // + // ASYMMETRY (deliberate, test-pinned): when fresh.hasMore is false the + // listing is complete, so the leftovers are DROPPED rather than + // appended. The append order - fresh first, leftovers after - is + // asserted too. + if (fresh.hasMore && existingByUri.isNotEmpty) { + mergedReplies.addAll(existingByUri.values); + } + + // ASYMMETRY (deliberate, test-pinned): repliesCursor is carried over + // from existing but hasMore is NOT - it comes from fresh, via + // copyWith's untouched field. That is the opposite pairing to the + // truncation branch above. It only shows at nested depth: for the node + // the request was anchored at, subtreeFromResponse below overwrites + // both from the response cursor, hiding whichever pairing was chosen. + 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, + ); + } + + /// The subtree a load-more-replies response resolves to. + /// + /// [fresh] is the response's anchored node, [existingNode] the version + /// already in the tree (null when it is not there), [requestCursor] the + /// cursor that was SENT and [responseCursor] the one that came back. + /// + /// [requestCursor] and [existingNode] are two independent observations + /// that the caller deliberately makes at different times - the cursor + /// before the fetch, the node after it - so they can disagree when a + /// concurrent refetch drops the node mid-flight. This function must treat + /// them as independent and does: that case is pinned by a test, and it + /// falls to the first-page branch even though a cursor was sent. The + /// temptation to collapse the two into one lookup lives at the call site + /// in `CommentsProvider._doLoadMoreReplies`, which carries the matching + /// warning. + static ThreadViewComment subtreeFromResponse({ + required ThreadViewComment fresh, + required ThreadViewComment? existingNode, + required String? requestCursor, + required String? responseCursor, + }) { + 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), + ); + return fresh.copyWith( + replies: [...existingReplies, ...newPage], + hasMore: responseCursor != null, + repliesCursor: responseCursor, + ); + } + + // 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); + return merged.copyWith( + hasMore: responseCursor != null, + repliesCursor: responseCursor, + ); + } +} diff --git a/lib/providers/comments_provider.dart b/lib/providers/comments_provider.dart index b12cdc9..6d5acb7 100644 --- a/lib/providers/comments_provider.dart +++ b/lib/providers/comments_provider.dart @@ -3,10 +3,12 @@ import 'dart:async' show Completer, Timer, unawaited; import 'package:characters/characters.dart'; import 'package:flutter/foundation.dart'; import '../models/comment.dart'; +import '../models/comment_thread_tree.dart'; import '../models/post.dart'; import '../services/api_exceptions.dart'; import '../services/comment_service.dart'; import '../services/coves_api_service.dart'; +import '../services/viewer_state_hydrator.dart'; import 'auth_provider.dart'; import 'vote_provider.dart'; @@ -20,23 +22,30 @@ import 'vote_provider.dart'; /// IMPORTANT: Provider instances are managed by CommentsProviderCache which /// handles LRU eviction and sign-out cleanup. Do not create directly in widgets. /// -/// IMPORTANT: Accepts AuthProvider reference to fetch fresh access -/// tokens before each authenticated request (critical for atProto OAuth -/// token rotation). +/// IMPORTANT: Accepts an AuthProvider so viewer-state hydration can tell +/// signed-in from signed-out. Fresh access tokens for the requests +/// themselves come from the shared CovesApiService's token callbacks. class CommentsProvider with ChangeNotifier { CommentsProvider( - this._authProvider, { + AuthProvider authProvider, { required String postUri, required String postCid, required CovesApiService apiService, VoteProvider? voteProvider, CommentService? commentService, List? indexingRetryDelays, + ViewerStateHydrator? hydrator, }) : _postUri = postUri, _postCid = postCid, _apiService = apiService, _voteProvider = voteProvider, _commentService = commentService, + _hydrator = + hydrator ?? + ViewerStateHydrator( + authProvider: authProvider, + voteProvider: voteProvider, + ), _indexingRetryDelays = indexingRetryDelays ?? _defaultIndexingRetryDelays; @@ -56,9 +65,14 @@ class CommentsProvider with ChangeNotifier { Duration(milliseconds: 1200), ]; - final AuthProvider _authProvider; final CovesApiService _apiService; final VoteProvider? _voteProvider; + + /// Seeds vote state from each comments response. Injected by the cache + /// that builds these providers; when omitted, built from the raw vote + /// provider this constructor still accepts. + final ViewerStateHydrator _hydrator; + final CommentService? _commentService; final List _indexingRetryDelays; @@ -68,6 +82,13 @@ class CommentsProvider with ChangeNotifier { // Comment state List _comments = []; + + /// The current thread as a value tree, for lookup and node replacement. + /// + /// Cheap to build per use: [CommentThreadTree] wraps [_comments] by + /// reference instead of copying it. + CommentThreadTree get _tree => CommentThreadTree(_comments); + bool _isLoading = false; bool _isLoadingMore = false; bool _isQuietLoading = false; @@ -310,9 +331,7 @@ class CommentsProvider with ChangeNotifier { // comments already on screen (a duplicate across pages keeps its // optimistic vote), so refresh and pagination share one path - on // refresh _comments is response.comments anyway. - if (_authProvider.isAuthenticated && _voteProvider != null) { - response.comments.forEach(_applyCommentVoteState); - } + _hydrator.hydrateCommentTree(response.comments); // Start time updates when comments are loaded if (_comments.isNotEmpty && _timeUpdateTimer == null) { @@ -421,7 +440,12 @@ class CommentsProvider with ChangeNotifier { // 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; + // + // Captured BEFORE the fetch, while the existing node is looked up AFTER + // it: a concurrent refetch can drop the node while this page is in + // flight, and the two observations are then allowed to disagree. Pinned + // by a test - do not collapse them into one lookup. + final requestCursor = _tree.findByUri(commentUri)?.repliesCursor; try { final response = await _apiService.getComments( @@ -446,17 +470,22 @@ class CommentsProvider with ChangeNotifier { return null; } - final existingNode = _findNodeByUri(commentUri); + final existingNode = _tree.findByUri(commentUri); if (response.comments.isEmpty) { // Nothing to load - clear the node's pagination state so the // "load more" affordance disappears instead of spinning forever. + // + // Deliberately BEFORE the anchoring guard below, and with no + // changed-check of its own: the outer condition is what keeps this + // a genuine no-op for a node with nothing to clear. if (existingNode != null && (existingNode.hasMore || existingNode.repliesCursor != null)) { - _comments = _replaceNode( - _comments, - existingNode.copyWith(hasMore: false, repliesCursor: null), + final cleared = existingNode.copyWith( + hasMore: false, + repliesCursor: null, ); + _comments = _tree.replaceNode(cleared).tree.nodes; } return null; } @@ -476,44 +505,29 @@ class CommentsProvider with ChangeNotifier { // 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 subtree = CommentThreadTree.subtreeFromResponse( + fresh: fresh, + existingNode: existingNode, + requestCursor: requestCursor, + responseCursor: 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. + final merge = _tree.replaceNode(subtree); + if (merge.replaced) { + _comments = merge.tree.nodes; + } else { + // The walk changed nothing. Usually that means the node is not in + // the top-level tree at all (e.g. below the depth cap, when the + // focused thread screen calls this) - but the merge is also a no-op + // if the subtree were already the instance sitting there, and the + // walk cannot tell the two apart. Either way the returned subtree + // is still useful to the caller. if (kDebugMode) { debugPrint( - 'ℹ️ loadMoreReplies: $commentUri not in top-level tree - ' - 'returning subtree without merging', + 'ℹ️ loadMoreReplies: nothing in the top-level tree changed for ' + '$commentUri - returning the subtree unmerged', ); } - } else { - _comments = updated; } // Apply viewer vote state from [fresh] - the nodes this response @@ -523,9 +537,7 @@ class CommentsProvider with ChangeNotifier { // confirmed through another surface (they were applied when their // own response arrived, which is enough). Nodes with an optimistic // vote the server has not indexed yet are protected either way. - if (_authProvider.isAuthenticated && _voteProvider != null) { - _applyCommentVoteState(fresh); - } + _hydrator.hydrateCommentTree([fresh]); if (kDebugMode) { debugPrint( @@ -544,102 +556,6 @@ class CommentsProvider with ChangeNotifier { } } - /// 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; - } - - /// Returns a copy of [nodes] with the node matching [replacement]'s URI - /// 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, - ) { - 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 /// /// Updates the sort option and triggers a refresh of comments. @@ -805,7 +721,7 @@ class CommentsProvider with ChangeNotifier { // 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)) { + _tree.containsUri(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 @@ -814,7 +730,7 @@ class CommentsProvider with ChangeNotifier { var attempt = 0; while (!_isDisposed && attempt < _indexingRetryDelays.length && - !_treeContainsUri(_comments, response.uri)) { + !_tree.containsUri(response.uri)) { await Future.delayed(_indexingRetryDelays[attempt]); attempt++; if (_isDisposed) { @@ -828,7 +744,7 @@ class CommentsProvider with ChangeNotifier { // so the new reply is merged into the tree at its correct position. if (parentComment != null && !_isDisposed && - !_treeContainsUri(_comments, response.uri)) { + !_tree.containsUri(response.uri)) { try { await loadMoreReplies(parentComment.comment.uri); } on Exception catch (e) { @@ -865,11 +781,6 @@ 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) => - 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 @@ -923,26 +834,6 @@ class CommentsProvider with ChangeNotifier { } } - /// Apply vote state for a comment and its replies recursively - /// - /// Extracts viewer vote data from comment and hands it to VoteProvider, - /// which decides whether the snapshot wins. Handles nested replies - /// recursively. - /// - /// IMPORTANT: Always applies the snapshot, even when viewer.vote is null. - /// This ensures that if a user removed their vote on another device, the - /// local state is cleared on refresh. - void _applyCommentVoteState(ThreadViewComment threadComment) { - final viewer = threadComment.comment.viewer; - _voteProvider!.applyServerVoteState( - postUri: threadComment.comment.uri, - voteDirection: viewer?.vote, - voteUri: viewer?.voteUri, - ); - - threadComment.replies?.forEach(_applyCommentVoteState); - } - /// Retry loading after error Future retry() async { _error = null; diff --git a/lib/providers/multi_feed_provider.dart b/lib/providers/multi_feed_provider.dart index e4342a2..b3cabb4 100644 --- a/lib/providers/multi_feed_provider.dart +++ b/lib/providers/multi_feed_provider.dart @@ -4,6 +4,7 @@ import 'package:flutter/foundation.dart'; import '../models/feed_state.dart'; import '../models/post.dart'; import '../services/coves_api_service.dart'; +import '../services/viewer_state_hydrator.dart'; import 'auth_provider.dart'; import 'community_subscription_provider.dart'; import 'vote_provider.dart'; @@ -26,13 +27,19 @@ enum FeedType { /// and must not be disposed here. class MultiFeedProvider with ChangeNotifier { MultiFeedProvider( - this._authProvider, { + AuthProvider authProvider, { required CovesApiService apiService, VoteProvider? voteProvider, CommunitySubscriptionProvider? subscriptionProvider, - }) : _apiService = apiService, - _voteProvider = voteProvider, - _subscriptionProvider = subscriptionProvider { + ViewerStateHydrator? hydrator, + }) : _authProvider = authProvider, + _apiService = apiService, + _hydrator = hydrator ?? + ViewerStateHydrator( + authProvider: authProvider, + voteProvider: voteProvider, + subscriptionProvider: subscriptionProvider, + ) { // Track initial auth state _wasAuthenticated = _authProvider.isAuthenticated; @@ -73,8 +80,11 @@ class MultiFeedProvider with ChangeNotifier { final AuthProvider _authProvider; final CovesApiService _apiService; - final VoteProvider? _voteProvider; - final CommunitySubscriptionProvider? _subscriptionProvider; + + /// Seeds vote/subscription state from each response. Injected app-wide; + /// when omitted, built from the raw vote/subscription providers this + /// constructor still accepts. + final ViewerStateHydrator _hydrator; // Track previous auth state to detect transitions bool _wasAuthenticated = false; @@ -279,34 +289,18 @@ class MultiFeedProvider with ChangeNotifier { debugPrint('✅ $feedName loaded: ${newPosts.length} posts total'); } - // Apply viewer vote state from the feed response for ALL items, - // including those with a null viewer.vote - the provider decides - // whether the snapshot may win, and a null direction is how a vote - // removed on another device gets cleared here. - if (_authProvider.isAuthenticated && _voteProvider != null) { - for (final feedItem in response.feed) { - final viewer = feedItem.post.viewer; - _voteProvider.applyServerVoteState( - postUri: feedItem.post.uri, - voteDirection: viewer?.vote, - voteUri: viewer?.voteUri, - ); - } - } - - // Initialize subscription state from community viewer data - // This ensures the menu shows correct subscribe/unsubscribe state - if (_authProvider.isAuthenticated && _subscriptionProvider != null) { - for (final feedItem in response.feed) { - final communityViewer = feedItem.post.community.viewer; - if (communityViewer?.subscribed != null) { - _subscriptionProvider.setInitialSubscriptionState( - communityDid: feedItem.post.community.did, - isSubscribed: communityViewer!.subscribed!, - ); - } - } - } + // Seed votes and subscription state from the viewer data this + // response carried. + // + // The RAW response feed is passed on purpose, not the merged + // `newPosts`: on cursor drift a page can re-deliver a post already on + // screen, and this site hydrates that duplicate (the + // CursorPaginationController-backed sites drop it before hydrating). + // Hydration also sits inside the try above, so a throw here lands on + // the feed's error state - unlike the controller sites, which keep + // the page and report. Both differences are deliberate and belong to + // this caller, not to the hydrator. + _hydrator.hydrateFeed(response.feed); } on Exception catch (e) { // SECURITY: Also check session change in error path to prevent // leaking stale data when a fetch fails after sign-out diff --git a/lib/providers/user_profile_provider.dart b/lib/providers/user_profile_provider.dart index feea811..db22ede 100644 --- a/lib/providers/user_profile_provider.dart +++ b/lib/providers/user_profile_provider.dart @@ -10,6 +10,7 @@ import '../models/user_profile.dart'; import '../services/api_exceptions.dart'; import '../services/comment_service.dart'; import '../services/coves_api_service.dart'; +import '../services/viewer_state_hydrator.dart'; import '../utils/cursor_pagination_controller.dart'; import 'auth_provider.dart'; import 'vote_provider.dart'; @@ -28,10 +29,16 @@ class UserProfileProvider with ChangeNotifier { required CovesApiService apiService, required CommentService commentService, VoteProvider? voteProvider, + ViewerStateHydrator? hydrator, }) : _authProvider = authProvider, _apiService = apiService, _commentService = commentService, - _voteProvider = voteProvider { + _hydrator = + hydrator ?? + ViewerStateHydrator( + authProvider: authProvider, + voteProvider: voteProvider, + ) { // The two feeds are the same cursor-pagination state machine with // different fetchers; the controllers own items/cursor/loading/errors // and this provider projects them onto the FeedState / CommentsState @@ -78,15 +85,31 @@ class UserProfileProvider with ChangeNotifier { } AuthProvider _authProvider; - final VoteProvider? _voteProvider; + + /// Seeds vote state from each page this provider loads. Injected app-wide + /// (where it also carries a subscription provider, which this surface + /// deliberately never uses - see [_hydratePostVotes]); when omitted, built + /// from the raw vote provider this constructor still accepts. + /// + /// Rebound whenever [_authProvider] changes - see [updateAuthProvider]. + ViewerStateHydrator _hydrator; + final CommentService _commentService; /// Update auth provider reference (called by ChangeNotifierProxyProvider) + /// + /// The hydrator is rebound to the new instance, not just the listener. + /// Its signed-in gate closes over whichever AuthProvider it was built + /// with, and this provider's hydration used to consult `_authProvider` + /// at call time - so leaving a stale hydrator here would silently keep + /// gating on the previous session. main.dart asserts the instance never + /// actually changes, but that assert is stripped in release. void updateAuthProvider(AuthProvider newAuth) { if (_authProvider != newAuth) { _authProvider.removeListener(_onAuthChanged); _authProvider = newAuth; _authProvider.addListener(_onAuthChanged); + _hydrator = _hydrator.withAuthProvider(newAuth); } } @@ -322,18 +345,16 @@ class UserProfileProvider with ChangeNotifier { /// Apply viewer vote state so a liked post shows a lit heart even when /// the profile is its first surface this session. + /// + /// Votes ONLY: these feed items carry `community.viewer.subscribed` too, + /// and this surface has never seeded it. `hydrateFeedVotesOnly` keeps that + /// a deliberate choice even when the injected hydrator does know about + /// subscriptions. + /// + /// The controller hands over only the deduplicated new items, so a + /// cursor-drift duplicate's stale snapshot never lands here. Future _hydratePostVotes(List newPosts) async { - final voteProvider = _voteProvider; - if (!_authProvider.isAuthenticated || voteProvider == null) return; - - for (final feedItem in newPosts) { - final viewer = feedItem.post.viewer; - voteProvider.applyServerVoteState( - postUri: feedItem.post.uri, - voteDirection: viewer?.vote, - voteUri: viewer?.voteUri, - ); - } + _hydrator.hydrateFeedVotesOnly(newPosts); } String _postsErrorMessage(Object error) { @@ -417,19 +438,11 @@ class UserProfileProvider with ChangeNotifier { /// Apply viewer vote state from the comments response. Safe on both /// refresh and pagination: the vote provider keeps an optimistic vote the /// appview has not indexed yet instead of adopting a stale snapshot. + /// + /// Actor comments come back as a flat list, so the flat traversal is the + /// right one here - there are no nested replies to recurse into. Future _hydrateCommentVotes(List newComments) async { - if (!_authProvider.isAuthenticated) return; - - if (_voteProvider == null) { - if (kDebugMode) { - debugPrint( - '⚠️ VoteProvider is null - cannot apply comment vote states', - ); - } - return; - } - - newComments.forEach(_applyCommentVoteState); + _hydrator.hydrateComments(newComments); } String _commentsErrorMessage(Object error) { @@ -492,28 +505,6 @@ class UserProfileProvider with ChangeNotifier { } } - /// Apply vote state for a comment from viewer data. - /// - /// Unlike CommentsProvider._applyCommentVoteState, this handles - /// flat CommentView objects (no nested replies) since actor comments - /// are returned as a flat list. - /// - /// If [_voteProvider] is null, this method returns early as a defensive - /// measure. A null viewer vote is still applied (vote removed on another - /// device) - the provider clears the local state unless it is protecting - /// an optimistic vote of its own. - void _applyCommentVoteState(CommentView comment) { - final voteProvider = _voteProvider; - if (voteProvider == null) return; - - final viewer = comment.viewer; - voteProvider.applyServerVoteState( - postUri: comment.uri, - voteDirection: viewer?.vote, - voteUri: viewer?.voteUri, - ); - } - /// Clear current profile and reset state void clearProfile() { _profile = null; diff --git a/lib/screens/community/community_feed_screen.dart b/lib/screens/community/community_feed_screen.dart index 15d74f6..d540d90 100644 --- a/lib/screens/community/community_feed_screen.dart +++ b/lib/screens/community/community_feed_screen.dart @@ -15,6 +15,7 @@ import '../../providers/community_subscription_provider.dart'; import '../../providers/vote_provider.dart'; import '../../services/api_exceptions.dart'; import '../../services/coves_api_service.dart'; +import '../../services/viewer_state_hydrator.dart'; import '../../utils/cursor_pagination_controller.dart'; import '../../utils/display_utils.dart'; import '../../utils/error_messages.dart'; @@ -191,16 +192,20 @@ class _CommunityFeedScreenState extends State { _isLoadingCommunity = false; }); - // Initialize subscription state from community viewer data + // Seed subscription state from this community's viewer data. + // + // The providers are looked up only once the snapshot is known to + // say something, so a provider-less tree is never asked for them. + // The hydrator repeats both checks; it skips a null `subscribed` + // rather than coercing it to false, unlike the discovery LIST site. final authProvider = context.read(); if (authProvider.isAuthenticated && community.viewer?.subscribed != null) { - final subscriptionProvider = - context.read(); - subscriptionProvider.setInitialSubscriptionState( - communityDid: community.did, - isSubscribed: community.viewer!.subscribed!, - ); + ViewerStateHydrator( + authProvider: authProvider, + subscriptionProvider: + context.read(), + ).hydrateCommunitySubscription(community); } } } catch (e) { @@ -257,6 +262,16 @@ class _CommunityFeedScreenState extends State { ); } + /// Seeds votes and subscription state for a landed page. + /// + /// [posts] is whatever the controller deduplicated down to - a + /// cursor-drift duplicate never reaches here, unlike MultiFeedProvider, + /// which hydrates straight off the raw response. The controller also + /// notifies before calling this and catches whatever it throws, keeping + /// the page on screen. Both belong to the controller, not the hydrator. + /// + /// The providers are read only after the auth gate so provider-less trees + /// never look them up. Future _syncViewerStates(List posts) async { if (!mounted) { return; @@ -265,25 +280,11 @@ class _CommunityFeedScreenState extends State { final authProvider = context.read(); if (!authProvider.isAuthenticated) return; - final voteProvider = context.read(); - final subscriptionProvider = context.read(); - - for (final post in posts) { - final viewer = post.post.viewer; - voteProvider.applyServerVoteState( - postUri: post.post.uri, - voteDirection: viewer?.vote, - voteUri: viewer?.voteUri, - ); - - final communityViewer = post.post.community.viewer; - if (communityViewer?.subscribed != null) { - subscriptionProvider.setInitialSubscriptionState( - communityDid: post.post.community.did, - isSubscribed: communityViewer!.subscribed!, - ); - } - } + ViewerStateHydrator( + authProvider: authProvider, + voteProvider: context.read(), + subscriptionProvider: context.read(), + ).hydrateFeed(posts); } Future _onRefresh() async { diff --git a/lib/screens/home/communities_admin_panel.dart b/lib/screens/home/communities_admin_panel.dart index 414f531..bc0bbcf 100644 --- a/lib/screens/home/communities_admin_panel.dart +++ b/lib/screens/home/communities_admin_panel.dart @@ -9,10 +9,9 @@ import '../../models/community.dart'; import '../../models/picked_image.dart'; import '../../services/api_exceptions.dart'; import '../../services/coves_api_service.dart'; -import '../../utils/image_crop_utils.dart'; -import '../../utils/image_picker_utils.dart'; -import '../../widgets/community_avatar.dart'; -import '../../widgets/image_source_picker.dart'; +import '../../utils/community_name_validator.dart'; +import 'community_avatar_upload_page.dart'; +import 'create_community_form.dart'; /// Admin handles that can create communities const Set kAdminHandles = { @@ -21,9 +20,6 @@ const Set kAdminHandles = { 'mari.local.coves.dev', // Local development account }; -/// Regex for DNS-valid community names (lowercase alphanumeric and hyphens) -final RegExp _dnsNameRegex = RegExp(r'^[a-z0-9]([a-z0-9-]*[a-z0-9])?$'); - /// Admin panel pages enum AdminPage { menu, @@ -36,6 +32,11 @@ enum AdminPage { /// Provides admin-only functionality: /// - Community creation form /// - Profile picture management +/// +/// A shell, not a screen: it owns which [AdminPage] is showing, the AppBar +/// that titles it, and every piece of state that has to OUTLIVE a page — +/// the community list, the create-form draft and the created-community +/// receipts. Page-local, genuinely transient state stays in the pages. class CommunitiesAdminPanel extends StatefulWidget { const CommunitiesAdminPanel({super.key}); @@ -44,175 +45,256 @@ class CommunitiesAdminPanel extends StatefulWidget { } class _CommunitiesAdminPanelState extends State { - // Current admin page AdminPage _currentPage = AdminPage.menu; - // Form controllers for create community - final TextEditingController _nameController = TextEditingController(); - final TextEditingController _displayNameController = TextEditingController(); - final TextEditingController _descriptionController = TextEditingController(); - - // Form controllers for change profile pic - final TextEditingController _communityHandleController = - TextEditingController(); - // Shared app-wide API client (owned by main.dart) — do not dispose here late final CovesApiService _apiService; - // Form state - bool _isSubmitting = false; - String? _nameError; - List _createdCommunities = []; - - // Profile pic state + // Community list for the profile-pic page, and the guard for its fetch. + // + // Held HERE rather than in CommunityAvatarUploadPage because that widget + // is destroyed every time the user returns to the menu, and both of these + // have to outlive it: + // + // * _isLoadingCommunities so that leaving and re-entering DURING a fetch + // does not fire a second one (test-pinned). Note the narrowness of + // that guarantee: once the fetch settles the flag is false again, and + // _navigateToPage calls _loadCommunities() unconditionally, so every + // later visit does refetch. Only the in-flight window is protected. + // * _communities so the already-fetched list is still on screen when the + // admin comes back, instead of the page starting empty each time. bool _isLoadingCommunities = false; List _communities = []; - CommunityView? _selectedCommunity; - PickedImage? _selectedImage; - // Computed state - bool get _isFormValid { - return _nameController.text.trim().isNotEmpty && - _displayNameController.text.trim().isNotEmpty && - _descriptionController.text.trim().isNotEmpty; - } + // The create form's draft and its receipt list. + // + // Held HERE for the same reason as the community list: CreateCommunityForm + // is only built while _currentPage is createCommunity, so anything it + // owned would be thrown away the moment the admin glanced at the menu. + // Both a half-typed draft and the receipts have to survive that trip. + // Test-pinned. + // + // The receipts carry the most weight: _createCommunity below runs on this + // State, so a create started here still finishes and still appends while + // the admin is off looking at the menu. That receipt is then the only + // record they have of the new community's handle and DID - nothing else + // in this panel shows it. + // + // This State owns them and therefore disposes them; the form only borrows + // them (see the listener note in CreateCommunityForm). + final TextEditingController _nameController = TextEditingController(); + final TextEditingController _displayNameController = TextEditingController(); + final TextEditingController _descriptionController = TextEditingController(); + List _createdCommunities = []; - // Generate handle preview from name - String get _handlePreview { - final name = _nameController.text.trim().toLowerCase(); - if (name.isEmpty) return '@c-{name}.coves.social'; - return '@c-$name.coves.social'; - } + // In-flight flags for the two write requests. + // + // Here for the same reason again, and this one is the sharpest: a request + // started on a page that is then left behind must still land somewhere. + // Owned by a State that dies with the page, a succeeded create would + // record no receipt, clear no draft and say nothing - and the re-entered + // page, seeing a fresh `false`, would happily let the admin submit the + // same community twice. Test-pinned. + bool _isCreatingCommunity = false; + bool _isUploadingAvatar = false; @override void initState() { super.initState(); _apiService = context.read(); - _nameController.addListener(_onTextChanged); - _displayNameController.addListener(_onTextChanged); - _descriptionController.addListener(_onTextChanged); } @override void dispose() { - // Remove listeners before disposing controllers - _nameController.removeListener(_onTextChanged); - _displayNameController.removeListener(_onTextChanged); - _descriptionController.removeListener(_onTextChanged); + // Owner disposes. The form removes its own listener in its dispose, + // which always runs first: the body is torn down before this State is. _nameController.dispose(); _displayNameController.dispose(); _descriptionController.dispose(); - _communityHandleController.dispose(); super.dispose(); } - void _onTextChanged() { - // Clear name error when user types - if (_nameError != null) { - setState(() { - _nameError = null; - }); - } else { - setState(() {}); - } - } - - /// Validates the community name is DNS-valid - bool _validateName() { - final name = _nameController.text.trim().toLowerCase(); - - if (name.isEmpty) { - setState(() => _nameError = 'Name is required'); - return false; - } - - if (name.length > 63) { - setState(() => _nameError = 'Name must be 63 characters or less'); - return false; - } - - if (!_dnsNameRegex.hasMatch(name)) { - setState(() { - _nameError = - 'Name must be lowercase letters, numbers, and hyphens only'; - }); - return false; - } - - setState(() => _nameError = null); - return true; - } - + /// Creates a community from the draft the controllers hold. + /// + /// The form has already validated the name; this owns the request and + /// everything that happens to its result. Note the shape of the error + /// handling: only the API call sits inside `try`. The success path - the + /// receipt, the draft clear, the confirmation - runs AFTER it, outside + /// every catch, because the community exists on the server by then and an + /// [Error] thrown while rendering a SnackBar must never be reported to the + /// admin as "creating the community failed". Future _createCommunity() async { - if (!_isFormValid || _isSubmitting) return; - - // Validate DNS-valid name before API call - if (!_validateName()) return; + if (_isCreatingCommunity) { + return; + } setState(() { - _isSubmitting = true; + _isCreatingCommunity = true; }); + final CreateCommunityResponse response; try { - final response = await _apiService.createCommunity( - name: _nameController.text.trim().toLowerCase(), + response = await _apiService.createCommunity( + name: CommunityNameValidator.normalize(_nameController.text), displayName: _displayNameController.text.trim(), description: _descriptionController.text.trim(), ); - - if (mounted) { - setState(() { - _createdCommunities = [..._createdCommunities, response]; - _isSubmitting = false; - }); - - // Clear form - _nameController.clear(); - _displayNameController.clear(); - _descriptionController.clear(); - - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Community created: ${response.handle}'), - backgroundColor: Colors.green[700], - behavior: SnackBarBehavior.floating, - ), - ); - } } on ApiException catch (e) { if (kDebugMode) { debugPrint('API error creating community: ${e.message}'); } - if (mounted) { - setState(() { - _isSubmitting = false; - }); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Failed to create community: ${e.message}'), - backgroundColor: Colors.red[700], - behavior: SnackBarBehavior.floating, - ), - ); - } + _finishCreate( + message: 'Failed to create community: ${e.message}', + background: Colors.red[700], + ); + return; } catch (e, stackTrace) { if (kDebugMode) { debugPrint('Unexpected error in _createCommunity: $e'); debugPrint('Stack trace: $stackTrace'); } - if (mounted) { - setState(() { - _isSubmitting = false; - }); - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('An unexpected error occurred. Please try again.'), - backgroundColor: Colors.red, - behavior: SnackBarBehavior.floating, - ), + _finishCreate( + message: 'An unexpected error occurred. Please try again.', + background: Colors.red, + ); + return; + } + + if (!mounted) { + return; + } + setState(() { + _isCreatingCommunity = false; + _createdCommunities = [..._createdCommunities, response]; + }); + + // Clear the draft that produced it, so a returning admin is not looking + // at a form that invites the same submission again. + _nameController.clear(); + _displayNameController.clear(); + _descriptionController.clear(); + + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text('Community created: ${response.handle}'), + backgroundColor: Colors.green[700], + behavior: SnackBarBehavior.floating, + ), + ); + } + + /// Clears the in-flight flag and reports a failed create. + void _finishCreate({required String message, required Color? background}) { + if (!mounted) { + return; + } + setState(() { + _isCreatingCommunity = false; + }); + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text(message), + backgroundColor: background, + behavior: SnackBarBehavior.floating, + ), + ); + } + + /// Uploads [image] as [community]'s avatar, returning whether it worked. + /// + /// Owned here rather than on the upload page for the same reason as + /// [_createCommunity]: a success that lands after the admin navigated away + /// still has to refresh the list, or the stale avatar survives on screen + /// and invites a pointless re-upload. Same error-handling shape too — the + /// API call alone is inside `try`. + Future _uploadAvatar({ + required CommunityView community, + required PickedImage image, + }) async { + if (_isUploadingAvatar) { + return false; + } + + setState(() { + _isUploadingAvatar = true; + }); + + try { + if (kDebugMode) { + debugPrint( + 'Uploading image: ${image.bytes.length} bytes, ${image.mimeType}', ); } + await _apiService.updateCommunity( + communityDid: community.did, + imageBytes: image.bytes, + mimeType: image.mimeType, + ); + } on ApiException catch (e, stackTrace) { + developer.log( + 'API error uploading avatar', + name: 'CommunitiesAdminPanel', + error: e, + stackTrace: stackTrace, + level: 1000, + ); + _finishUpload( + message: 'Failed to upload avatar: ${e.message}', + background: Colors.red[700], + ); + return false; + } catch (e, stackTrace) { + developer.log( + 'Unexpected error uploading avatar', + name: 'CommunitiesAdminPanel', + error: e, + stackTrace: stackTrace, + level: 1000, + ); + _finishUpload( + message: 'An unexpected error occurred. Please try again.', + background: Colors.red, + ); + return false; } + + if (!mounted) { + return false; + } + setState(() { + _isUploadingAvatar = false; + }); + + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + 'Avatar updated for ${community.displayName ?? community.name}', + ), + backgroundColor: Colors.green[700], + behavior: SnackBarBehavior.floating, + ), + ); + + // Reload so the new avatar replaces the stale one in the picker. + await _loadCommunities(); + return true; + } + + /// Clears the in-flight flag and reports a failed upload. + void _finishUpload({required String message, required Color? background}) { + if (!mounted) { + return; + } + setState(() { + _isUploadingAvatar = false; + }); + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text(message), + backgroundColor: background, + behavior: SnackBarBehavior.floating, + ), + ); } String _getAdminTitle() { @@ -230,46 +312,30 @@ class _CommunitiesAdminPanelState extends State { setState(() { _currentPage = page; }); - // Load communities when navigating to profile pic page + // Load communities when navigating to profile pic page. Not awaited + // because this method is void and the fetch is safe to leave running: + // it catches its own errors and re-checks `mounted` before touching + // state, so nothing is lost by not holding on to its Future. if (page == AdminPage.changeProfilePic) { _loadCommunities(); } } + /// Returns to the menu. + /// + /// Nothing is reset here: the selected community and the picked image live + /// in [CommunityAvatarUploadPage], and swapping the body away destroys + /// that State, so re-entering always starts clean. Test-pinned. void _navigateBack() { setState(() { _currentPage = AdminPage.menu; - _selectedCommunity = null; - _selectedImage = null; }); } - @override - Widget build(BuildContext context) { - // NOTE: system back is handled at the shell level (MainShellScreen) and - // only intercepted when the Create tab has a draft; here it backgrounds - // the app as usual — use the in-app Back arrow to return to the menu. - return Scaffold( - backgroundColor: AppColors.background, - appBar: AppBar( - backgroundColor: AppColors.background, - foregroundColor: Colors.white, - title: Text(_getAdminTitle()), - automaticallyImplyLeading: false, - leading: _currentPage != AdminPage.menu - ? IconButton( - icon: const Icon(Icons.arrow_back), - tooltip: 'Back', - onPressed: _navigateBack, - ) - : null, - ), - body: _buildAdminUI(), - ); - } - Future _loadCommunities() async { - if (_isLoadingCommunities) return; + if (_isLoadingCommunities) { + return; + } setState(() { _isLoadingCommunities = true; @@ -308,18 +374,64 @@ class _CommunitiesAdminPanelState extends State { } } + @override + Widget build(BuildContext context) { + // NOTE: system back is handled at the shell level (MainShellScreen) and + // only intercepted when the Create tab has a draft; here it backgrounds + // the app as usual — use the in-app Back arrow to return to the menu. + // The pages below are deliberately NOT Navigator routes: real routes + // would let system back pop them and change that. + return Scaffold( + backgroundColor: AppColors.background, + appBar: AppBar( + backgroundColor: AppColors.background, + foregroundColor: Colors.white, + title: Text(_getAdminTitle()), + automaticallyImplyLeading: false, + leading: _currentPage != AdminPage.menu + ? IconButton( + icon: const Icon(Icons.arrow_back), + tooltip: 'Back', + onPressed: _navigateBack, + ) + : null, + ), + body: _buildAdminUI(), + ); + } + Widget _buildAdminUI() { switch (_currentPage) { case AdminPage.menu: - return _buildAdminMenu(); + return _AdminMenu(onSelectPage: _navigateToPage); case AdminPage.createCommunity: - return _buildCreateCommunityUI(); + return CreateCommunityForm( + nameController: _nameController, + displayNameController: _displayNameController, + descriptionController: _descriptionController, + createdCommunities: _createdCommunities, + isSubmitting: _isCreatingCommunity, + onSubmit: _createCommunity, + ); case AdminPage.changeProfilePic: - return _buildChangeProfilePicUI(); + return CommunityAvatarUploadPage( + communities: _communities, + isLoadingCommunities: _isLoadingCommunities, + isUploading: _isUploadingAvatar, + onUploadAvatar: _uploadAvatar, + ); } } +} + +/// The panel's landing page: one row per admin tool. +class _AdminMenu extends StatelessWidget { + const _AdminMenu({required this.onSelectPage}); + + final ValueChanged onSelectPage; - Widget _buildAdminMenu() { + @override + Widget build(BuildContext context) { return Padding( padding: const EdgeInsets.all(16), child: Column( @@ -339,30 +451,41 @@ class _CommunitiesAdminPanelState extends State { style: TextStyle(fontSize: 14, color: Color(0xFFB6C2D2)), ), const SizedBox(height: 24), - _buildAdminMenuItem( + _AdminMenuItem( icon: Icons.add_circle_outline, title: 'Create Community', subtitle: 'Create a new community for Coves users', - onTap: () => _navigateToPage(AdminPage.createCommunity), + onTap: () => onSelectPage(AdminPage.createCommunity), ), const SizedBox(height: 12), - _buildAdminMenuItem( + _AdminMenuItem( icon: Icons.image_outlined, title: 'Change Profile Pic', subtitle: 'Update a community\'s profile picture', - onTap: () => _navigateToPage(AdminPage.changeProfilePic), + onTap: () => onSelectPage(AdminPage.changeProfilePic), ), ], ), ); } +} + +/// One tappable tool row in [_AdminMenu]. +class _AdminMenuItem extends StatelessWidget { + const _AdminMenuItem({ + required this.icon, + required this.title, + required this.subtitle, + required this.onTap, + }); - Widget _buildAdminMenuItem({ - required IconData icon, - required String title, - required String subtitle, - required VoidCallback onTap, - }) { + final IconData icon; + final String title; + final String subtitle; + final VoidCallback onTap; + + @override + Widget build(BuildContext context) { return InkWell( onTap: onTap, borderRadius: BorderRadius.circular(12), @@ -407,750 +530,10 @@ class _CommunitiesAdminPanelState extends State { ], ), ), - const Icon( - Icons.chevron_right, - color: Color(0xFFB6C2D2), - ), - ], - ), - ), - ); - } - - Widget _buildCreateCommunityUI() { - return SingleChildScrollView( - padding: const EdgeInsets.all(16), - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - // Header - const Text( - 'Create Community', - style: TextStyle( - fontSize: 24, - color: Colors.white, - fontWeight: FontWeight.bold, - ), - ), - const SizedBox(height: 8), - const Text( - 'Create a new community for Coves users', - style: TextStyle(fontSize: 14, color: Color(0xFFB6C2D2)), - ), - const SizedBox(height: 24), - - // Name field (DNS-valid slug) - _buildTextField( - controller: _nameController, - label: 'Name (unique identifier)', - hint: 'worldnews', - helperText: 'DNS-valid, lowercase, no spaces', - errorText: _nameError, - ), - const SizedBox(height: 16), - - // Handle preview - Container( - padding: const EdgeInsets.all(12), - decoration: BoxDecoration( - color: AppColors.backgroundSecondary, - borderRadius: BorderRadius.circular(8), - border: Border.all(color: AppColors.border), - ), - child: Row( - children: [ - const Icon(Icons.link, color: AppColors.primary, size: 20), - const SizedBox(width: 8), - Expanded( - child: Text( - _handlePreview, - style: const TextStyle( - color: AppColors.primary, - fontFamily: 'monospace', - ), - ), - ), - ], - ), - ), - const SizedBox(height: 16), - - // Display Name field - _buildTextField( - controller: _displayNameController, - label: 'Display Name', - hint: 'World News', - helperText: 'Human-readable name shown in the UI', - ), - const SizedBox(height: 16), - - // Description field - _buildTextField( - controller: _descriptionController, - label: 'Description', - hint: 'Global news and current events from around the world', - maxLines: 3, - ), - const SizedBox(height: 24), - - // Create button - SizedBox( - width: double.infinity, - child: ElevatedButton( - onPressed: - _isFormValid && !_isSubmitting ? _createCommunity : null, - style: ElevatedButton.styleFrom( - backgroundColor: AppColors.primary, - foregroundColor: Colors.white, - padding: const EdgeInsets.symmetric(vertical: 16), - shape: RoundedRectangleBorder( - borderRadius: BorderRadius.circular(8), - ), - disabledBackgroundColor: AppColors.backgroundSecondary, - ), - child: _isSubmitting - ? const SizedBox( - height: 20, - width: 20, - child: CircularProgressIndicator( - strokeWidth: 2, - valueColor: AlwaysStoppedAnimation(Colors.white), - ), - ) - : const Text( - 'Create Community', - style: TextStyle( - fontSize: 16, - fontWeight: FontWeight.w600, - ), - ), - ), - ), - - // Created communities list - if (_createdCommunities.isNotEmpty) ...[ - const SizedBox(height: 32), - const Text( - 'Created Communities', - style: TextStyle( - fontSize: 18, - color: Colors.white, - fontWeight: FontWeight.bold, - ), - ), - const SizedBox(height: 12), - ..._createdCommunities - .map((community) => _buildCommunityTile(community)), - ], - ], - ), - ); - } - - Widget _buildChangeProfilePicUI() { - return SingleChildScrollView( - padding: const EdgeInsets.all(16), - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - const Text( - 'Change Profile Picture', - style: TextStyle( - fontSize: 24, - color: Colors.white, - fontWeight: FontWeight.bold, - ), - ), - const SizedBox(height: 8), - const Text( - 'Select a community and upload a new profile picture', - style: TextStyle(fontSize: 14, color: Color(0xFFB6C2D2)), - ), - const SizedBox(height: 24), - - // Community selector - const Text( - 'Select Community', - style: TextStyle( - color: Colors.white, - fontSize: 14, - fontWeight: FontWeight.w500, - ), - ), - const SizedBox(height: 8), - - if (_isLoadingCommunities) - const Center( - child: Padding( - padding: EdgeInsets.all(24), - child: CircularProgressIndicator( - valueColor: AlwaysStoppedAnimation(AppColors.primary), - ), - ), - ) - else if (_communities.isEmpty) - Container( - padding: const EdgeInsets.all(16), - decoration: BoxDecoration( - color: AppColors.backgroundSecondary, - borderRadius: BorderRadius.circular(8), - border: Border.all(color: AppColors.border), - ), - child: const Center( - child: Text( - 'No communities found', - style: TextStyle(color: Color(0xFFB6C2D2)), - ), - ), - ) - else - ...(_communities.map((community) => _buildCommunitySelectTile(community))), - - if (_selectedCommunity != null) ...[ - const SizedBox(height: 24), - - // Show current vs new image comparison when image is selected - if (_selectedImage != null) ...[ - const Text( - 'Preview Changes', - style: TextStyle( - color: Colors.white, - fontSize: 14, - fontWeight: FontWeight.w500, - ), - ), - const SizedBox(height: 12), - Row( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - // Current image - Column( - children: [ - Container( - width: 100, - height: 100, - decoration: BoxDecoration( - color: AppColors.backgroundSecondary, - borderRadius: BorderRadius.circular(50), - border: Border.all(color: AppColors.border, width: 2), - ), - child: CommunityAvatar( - name: _selectedCommunity!.name, - avatarUrl: _selectedCommunity!.avatar, - size: 100, - fallbackColor: AppColors.backgroundSecondary, - fallbackIcon: const Icon( - Icons.workspaces_outlined, - size: 40, - color: AppColors.primary, - ), - ), - ), - const SizedBox(height: 8), - const Text( - 'Current', - style: TextStyle( - color: Color(0xFFB6C2D2), - fontSize: 12, - ), - ), - ], - ), - const Padding( - padding: EdgeInsets.symmetric(horizontal: 16), - child: Icon( - Icons.arrow_forward, - color: AppColors.primary, - size: 24, - ), - ), - // New image preview - Column( - children: [ - Container( - width: 100, - height: 100, - decoration: BoxDecoration( - color: AppColors.backgroundSecondary, - borderRadius: BorderRadius.circular(50), - border: Border.all(color: AppColors.primary, width: 2), - ), - child: ClipRRect( - borderRadius: BorderRadius.circular(50), - child: Image.file( - _selectedImage!.file, - fit: BoxFit.cover, - ), - ), - ), - const SizedBox(height: 8), - const Text( - 'New', - style: TextStyle( - color: AppColors.primary, - fontSize: 12, - fontWeight: FontWeight.w600, - ), - ), - ], - ), - ], - ), - const SizedBox(height: 8), - Center( - child: Text( - _selectedCommunity!.displayName ?? _selectedCommunity!.name, - style: const TextStyle( - color: Colors.white, - fontSize: 16, - fontWeight: FontWeight.w600, - ), - ), - ), - Center( - child: Text( - '@${_selectedCommunity!.handle ?? _selectedCommunity!.name}', - style: const TextStyle( - color: Color(0xFFB6C2D2), - fontSize: 14, - ), - ), - ), - const SizedBox(height: 24), - - // Action buttons when image is selected - Row( - children: [ - // Clear button - Expanded( - child: OutlinedButton.icon( - onPressed: _clearSelectedImage, - icon: const Icon(Icons.close), - label: const Text('Clear'), - style: OutlinedButton.styleFrom( - foregroundColor: Colors.white, - side: const BorderSide(color: AppColors.border), - padding: const EdgeInsets.symmetric(vertical: 14), - shape: RoundedRectangleBorder( - borderRadius: BorderRadius.circular(8), - ), - ), - ), - ), - const SizedBox(width: 12), - // Upload button - Expanded( - flex: 2, - child: ElevatedButton.icon( - onPressed: _isSubmitting ? null : _uploadImage, - icon: const Icon(Icons.upload), - label: Text(_isSubmitting ? 'Uploading...' : 'Upload'), - style: ElevatedButton.styleFrom( - backgroundColor: AppColors.primary, - foregroundColor: Colors.white, - padding: const EdgeInsets.symmetric(vertical: 14), - shape: RoundedRectangleBorder( - borderRadius: BorderRadius.circular(8), - ), - disabledBackgroundColor: AppColors.backgroundSecondary, - ), - ), - ), - ], - ), - const SizedBox(height: 12), - // Select different image button - SizedBox( - width: double.infinity, - child: TextButton.icon( - onPressed: _pickAndUploadImage, - icon: const Icon(Icons.photo_library, size: 18), - label: const Text('Select Different Image'), - style: TextButton.styleFrom( - foregroundColor: AppColors.teal, - padding: const EdgeInsets.symmetric(vertical: 12), - ), - ), - ), - ] else ...[ - // Show current profile picture when no new image is selected - const Text( - 'Current Profile Picture', - style: TextStyle( - color: Colors.white, - fontSize: 14, - fontWeight: FontWeight.w500, - ), - ), - const SizedBox(height: 12), - Center( - child: Column( - children: [ - Container( - width: 120, - height: 120, - decoration: BoxDecoration( - color: AppColors.backgroundSecondary, - borderRadius: BorderRadius.circular(60), - border: Border.all(color: AppColors.border, width: 2), - ), - child: CommunityAvatar( - name: _selectedCommunity!.name, - avatarUrl: _selectedCommunity!.avatar, - size: 120, - fallbackColor: AppColors.backgroundSecondary, - fallbackIcon: const Icon( - Icons.workspaces_outlined, - size: 48, - color: AppColors.primary, - ), - ), - ), - const SizedBox(height: 8), - Text( - _selectedCommunity!.displayName ?? _selectedCommunity!.name, - style: const TextStyle( - color: Colors.white, - fontSize: 16, - fontWeight: FontWeight.w600, - ), - ), - Text( - '@${_selectedCommunity!.handle ?? _selectedCommunity!.name}', - style: const TextStyle( - color: Color(0xFFB6C2D2), - fontSize: 14, - ), - ), - ], - ), - ), - const SizedBox(height: 24), - - // Select image button - SizedBox( - width: double.infinity, - child: ElevatedButton.icon( - onPressed: _isSubmitting ? null : _pickAndUploadImage, - icon: const Icon(Icons.add_photo_alternate), - label: const Text('Select New Picture'), - style: ElevatedButton.styleFrom( - backgroundColor: AppColors.primary, - foregroundColor: Colors.white, - padding: const EdgeInsets.symmetric(vertical: 16), - shape: RoundedRectangleBorder( - borderRadius: BorderRadius.circular(8), - ), - disabledBackgroundColor: AppColors.backgroundSecondary, - ), - ), - ), - ], - ], - ], - ), - ); - } - - Widget _buildCommunitySelectTile(CommunityView community) { - final isSelected = _selectedCommunity?.did == community.did; - return GestureDetector( - onTap: () { - setState(() { - _selectedCommunity = community; - }); - }, - child: Container( - margin: const EdgeInsets.only(bottom: 8), - padding: const EdgeInsets.all(12), - decoration: BoxDecoration( - color: AppColors.backgroundSecondary, - borderRadius: BorderRadius.circular(8), - border: Border.all( - color: isSelected ? AppColors.primary : AppColors.border, - width: isSelected ? 2 : 1, - ), - ), - child: Row( - children: [ - CommunityAvatar( - name: community.name, - avatarUrl: community.avatar, - size: 40, - fallbackColor: AppColors.background, - fallbackIcon: const Icon( - Icons.workspaces_outlined, - size: 20, - color: AppColors.primary, - ), - ), - const SizedBox(width: 12), - Expanded( - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text( - community.displayName ?? community.name, - style: const TextStyle( - color: Colors.white, - fontWeight: FontWeight.w500, - ), - ), - Text( - '@${community.handle ?? community.name}', - style: const TextStyle( - color: Color(0xFFB6C2D2), - fontSize: 12, - ), - ), - ], - ), - ), - if (isSelected) - const Icon( - Icons.check_circle, - color: AppColors.primary, - size: 24, - ), + const Icon(Icons.chevron_right, color: Color(0xFFB6C2D2)), ], ), ), ); } - - Future _pickAndUploadImage() async { - // Show bottom sheet to choose between gallery and camera - final source = await ImageSourcePicker.show(context); - if (source == null) return; - - try { - // Pick image and open native cropper - final picked = await ImageCropUtils.pickAndCropImage( - source: source, - ); - if (picked != null && mounted) { - setState(() { - _selectedImage = picked; - }); - } - } on ImageValidationException catch (e) { - if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text(e.message), - backgroundColor: Colors.red[700], - behavior: SnackBarBehavior.floating, - ), - ); - } - } on Exception catch (e, stackTrace) { - developer.log( - 'Error picking image', - name: 'CommunitiesAdminPanel', - error: e, - stackTrace: stackTrace, - level: 1000, // Error level - ); - if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Failed to process image: ${e.toString()}'), - backgroundColor: Colors.red[700], - behavior: SnackBarBehavior.floating, - ), - ); - } - } - } - - Future _uploadImage() async { - if (_selectedImage == null || _selectedCommunity == null) { - return; - } - - setState(() { - _isSubmitting = true; - }); - - try { - // Use bytes and mimeType from PickedImage (already read during picking) - final imageBytes = _selectedImage!.bytes; - final mimeType = _selectedImage!.mimeType; - - if (kDebugMode) { - debugPrint( - 'Uploading image: ${imageBytes.length} bytes, $mimeType', - ); - } - - await _apiService.updateCommunity( - communityDid: _selectedCommunity!.did, - imageBytes: imageBytes, - mimeType: mimeType, - ); - - if (mounted) { - setState(() { - _isSubmitting = false; - _selectedImage = null; - }); - - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text( - 'Avatar updated for ${_selectedCommunity!.displayName ?? _selectedCommunity!.name}', - ), - backgroundColor: Colors.green[700], - behavior: SnackBarBehavior.floating, - ), - ); - - // Reload communities list to show updated avatar - await _loadCommunities(); - } - } on ApiException catch (e, stackTrace) { - developer.log( - 'API error uploading avatar', - name: 'CommunitiesAdminPanel', - error: e, - stackTrace: stackTrace, - level: 1000, - ); - if (mounted) { - setState(() { - _isSubmitting = false; - }); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Failed to upload avatar: ${e.message}'), - backgroundColor: Colors.red[700], - behavior: SnackBarBehavior.floating, - ), - ); - } - } catch (e, stackTrace) { - developer.log( - 'Unexpected error uploading avatar', - name: 'CommunitiesAdminPanel', - error: e, - stackTrace: stackTrace, - level: 1000, - ); - if (mounted) { - setState(() { - _isSubmitting = false; - }); - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('An unexpected error occurred. Please try again.'), - backgroundColor: Colors.red, - behavior: SnackBarBehavior.floating, - ), - ); - } - } - } - - void _clearSelectedImage() { - setState(() { - _selectedImage = null; - }); - } - - Widget _buildTextField({ - required TextEditingController controller, - required String label, - required String hint, - String? helperText, - String? errorText, - int maxLines = 1, - }) { - final hasError = errorText != null; - return Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text( - label, - style: const TextStyle( - color: Colors.white, - fontSize: 14, - fontWeight: FontWeight.w500, - ), - ), - const SizedBox(height: 8), - TextField( - controller: controller, - maxLines: maxLines, - style: const TextStyle(color: Colors.white), - decoration: InputDecoration( - hintText: hint, - hintStyle: TextStyle(color: Colors.white.withValues(alpha: 0.4)), - helperText: hasError ? null : helperText, - helperStyle: const TextStyle(color: Color(0xFFB6C2D2)), - errorText: errorText, - errorStyle: const TextStyle(color: Colors.red), - filled: true, - fillColor: AppColors.backgroundSecondary, - border: OutlineInputBorder( - borderRadius: BorderRadius.circular(8), - borderSide: const BorderSide(color: AppColors.border), - ), - enabledBorder: OutlineInputBorder( - borderRadius: BorderRadius.circular(8), - borderSide: BorderSide( - color: hasError ? Colors.red : AppColors.border, - ), - ), - focusedBorder: OutlineInputBorder( - borderRadius: BorderRadius.circular(8), - borderSide: BorderSide( - color: hasError ? Colors.red : AppColors.primary, - ), - ), - ), - ), - ], - ); - } - - Widget _buildCommunityTile(CreateCommunityResponse community) { - return Container( - margin: const EdgeInsets.only(bottom: 8), - padding: const EdgeInsets.all(12), - decoration: BoxDecoration( - color: AppColors.backgroundSecondary, - borderRadius: BorderRadius.circular(8), - border: Border.all(color: Colors.green.withValues(alpha: 0.3)), - ), - child: Row( - children: [ - const Icon(Icons.check_circle, color: Colors.green, size: 20), - const SizedBox(width: 12), - Expanded( - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text( - community.handle, - style: const TextStyle( - color: Colors.white, - fontWeight: FontWeight.w500, - ), - ), - Text( - community.did, - style: const TextStyle( - color: Color(0xFFB6C2D2), - fontSize: 12, - fontFamily: 'monospace', - ), - overflow: TextOverflow.ellipsis, - ), - ], - ), - ), - ], - ), - ); - } } diff --git a/lib/screens/home/communities_discovery_screen.dart b/lib/screens/home/communities_discovery_screen.dart index 0af1920..f45c5fd 100644 --- a/lib/screens/home/communities_discovery_screen.dart +++ b/lib/screens/home/communities_discovery_screen.dart @@ -12,6 +12,7 @@ import '../../providers/auth_provider.dart'; import '../../providers/community_subscription_provider.dart'; import '../../services/api_exceptions.dart'; import '../../services/coves_api_service.dart'; +import '../../services/viewer_state_hydrator.dart'; import '../../utils/community_search_utils.dart'; import '../../utils/responsive_utils.dart'; import '../../widgets/community_chip.dart'; @@ -162,21 +163,23 @@ class _CommunitiesDiscoveryScreenState } } + /// Only ever called from the authenticated branch of [_loadAllSections], + /// which is why the provider lookup below is unconditional. + /// + /// The LIST seeding skips a community whose `viewer` is null but coerces a + /// present viewer's null `subscribed` to false - the opposite of how the + /// single-community site treats that same input. Future _loadSubscribed() async { - // Capture provider before async gap to avoid context.read after await - final subProvider = context.read(); + // Capture providers before async gap to avoid context.read after await + final hydrator = ViewerStateHydrator( + authProvider: context.read(), + subscriptionProvider: context.read(), + ); await _loadSection( limit: 10, subscribed: true, onSuccess: (communities) { - for (final c in communities) { - if (c.viewer != null) { - subProvider.setInitialSubscriptionState( - communityDid: c.did, - isSubscribed: c.viewer!.subscribed ?? false, - ); - } - } + hydrator.hydrateCommunityListSubscriptions(communities); _subscribedCommunities = communities; }, onStateChange: (isLoading, error) { diff --git a/lib/screens/home/community_avatar_upload_page.dart b/lib/screens/home/community_avatar_upload_page.dart new file mode 100644 index 0000000..09f5b5b --- /dev/null +++ b/lib/screens/home/community_avatar_upload_page.dart @@ -0,0 +1,548 @@ +import 'dart:developer' as developer; + +import 'package:flutter/material.dart'; +import 'package:image_picker/image_picker.dart'; + +import '../../constants/app_colors.dart'; +import '../../models/community.dart'; +import '../../models/picked_image.dart'; +import '../../utils/image_crop_utils.dart'; +import '../../utils/image_picker_utils.dart'; +import '../../widgets/community_avatar.dart'; +import '../../widgets/image_source_picker.dart'; + +/// Picks and crops an image for real. The default for +/// [CommunityAvatarUploadPage.pickImage], hoisted to a top-level function so +/// it can be a const default value. +Future _pickAndCropImage(ImageSource source) => + ImageCropUtils.pickAndCropImage(source: source); + +/// The admin panel's "Change Profile Pic" page body. +/// +/// Owns the selection: which community, and which freshly picked image. It +/// deliberately does NOT own the community list, the upload request or the +/// in-flight flag — the panel shell owns all three, because this widget is +/// destroyed whenever the user returns to the menu and a request must be +/// able to land after that. +/// +/// Destroying this widget is also what clears the selected community and the +/// selected image on back-navigation - there is no explicit reset anywhere. +class CommunityAvatarUploadPage extends StatefulWidget { + const CommunityAvatarUploadPage({ + required this.communities, + required this.isLoadingCommunities, + required this.isUploading, + required this.onUploadAvatar, + this.pickImage = _pickAndCropImage, + super.key, + }); + + /// Communities the shell has loaded. Empty renders the empty state. + final List communities; + + /// Whether the shell's fetch is in flight; renders the spinner. + final bool isLoadingCommunities; + + /// Whether the shell has an avatar upload in flight. + final bool isUploading; + + /// Asks the shell to upload [PickedImage] as the community's avatar. + /// Resolves true when it succeeded. + /// + /// The shell owns the request so its result survives this page being + /// popped mid-flight; all this widget does with the answer is drop its + /// local preview. + final Future Function({ + required CommunityView community, + required PickedImage image, + }) onUploadAvatar; + + /// Seam for the platform image pick + crop, which is otherwise reachable + /// only through statics with no injection point. Defaults to the real + /// thing, so production behaviour is unchanged and only tests pass + /// anything else. + final Future Function(ImageSource source) pickImage; + + @override + State createState() => + _CommunityAvatarUploadPageState(); +} + +class _CommunityAvatarUploadPageState extends State { + CommunityView? _selectedCommunity; + PickedImage? _selectedImage; + + void _selectCommunity(CommunityView community) { + setState(() { + _selectedCommunity = community; + }); + } + + void _clearSelectedImage() { + setState(() { + _selectedImage = null; + }); + } + + Future _pickAndUploadImage() async { + // Show bottom sheet to choose between gallery and camera + final source = await ImageSourcePicker.show(context); + if (source == null) { + return; + } + + try { + // Pick image and open native cropper + final picked = await widget.pickImage(source); + if (picked != null && mounted) { + setState(() { + _selectedImage = picked; + }); + } + } on ImageValidationException catch (e) { + if (mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text(e.message), + backgroundColor: Colors.red[700], + behavior: SnackBarBehavior.floating, + ), + ); + } + } on Exception catch (e, stackTrace) { + developer.log( + 'Error picking image', + name: 'CommunityAvatarUploadPage', + error: e, + stackTrace: stackTrace, + level: 1000, // Error level + ); + if (mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text('Failed to process image: ${e.toString()}'), + backgroundColor: Colors.red[700], + behavior: SnackBarBehavior.floating, + ), + ); + } + } + } + + /// Hands the upload to the shell and drops the local preview if it + /// worked. + /// + /// Everything that must survive this page - the in-flight flag, the + /// confirmation, the list refresh - happens on the shell's side. Clearing + /// [_selectedImage] is the only part left here, and it is purely cosmetic: + /// if this State is gone by the time the answer arrives there is no + /// preview left to clear. + Future _uploadImage() async { + final community = _selectedCommunity; + final image = _selectedImage; + if (image == null || community == null) { + return; + } + + final succeeded = await widget.onUploadAvatar( + community: community, + image: image, + ); + if (succeeded && mounted) { + setState(() { + _selectedImage = null; + }); + } + } + + @override + Widget build(BuildContext context) { + final selected = _selectedCommunity; + return SingleChildScrollView( + padding: const EdgeInsets.all(16), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + const Text( + 'Change Profile Picture', + style: TextStyle( + fontSize: 24, + color: Colors.white, + fontWeight: FontWeight.bold, + ), + ), + const SizedBox(height: 8), + const Text( + 'Select a community and upload a new profile picture', + style: TextStyle(fontSize: 14, color: Color(0xFFB6C2D2)), + ), + const SizedBox(height: 24), + + // Community selector + const Text( + 'Select Community', + style: TextStyle( + color: Colors.white, + fontSize: 14, + fontWeight: FontWeight.w500, + ), + ), + const SizedBox(height: 8), + + if (widget.isLoadingCommunities) + const Center( + child: Padding( + padding: EdgeInsets.all(24), + child: CircularProgressIndicator( + valueColor: AlwaysStoppedAnimation(AppColors.primary), + ), + ), + ) + else if (widget.communities.isEmpty) + Container( + padding: const EdgeInsets.all(16), + decoration: BoxDecoration( + color: AppColors.backgroundSecondary, + borderRadius: BorderRadius.circular(8), + border: Border.all(color: AppColors.border), + ), + child: const Center( + child: Text( + 'No communities found', + style: TextStyle(color: Color(0xFFB6C2D2)), + ), + ), + ) + else + ...widget.communities.map( + (community) => _CommunitySelectTile( + community: community, + isSelected: selected?.did == community.did, + onTap: () => _selectCommunity(community), + ), + ), + + if (selected != null) ...[ + const SizedBox(height: 24), + if (_selectedImage != null) + ..._buildImageComparison(selected, _selectedImage!) + else + ..._buildCurrentPicture(selected), + ], + ], + ), + ); + } + + /// Current-vs-new preview plus the clear/upload/reselect actions, shown + /// once an image has been picked. + List _buildImageComparison( + CommunityView community, + PickedImage image, + ) { + return [ + const Text( + 'Preview Changes', + style: TextStyle( + color: Colors.white, + fontSize: 14, + fontWeight: FontWeight.w500, + ), + ), + const SizedBox(height: 12), + Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + // Current image + Column( + children: [ + Container( + width: 100, + height: 100, + decoration: BoxDecoration( + color: AppColors.backgroundSecondary, + borderRadius: BorderRadius.circular(50), + border: Border.all(color: AppColors.border, width: 2), + ), + child: CommunityAvatar( + name: community.name, + avatarUrl: community.avatar, + size: 100, + fallbackColor: AppColors.backgroundSecondary, + fallbackIcon: const Icon( + Icons.workspaces_outlined, + size: 40, + color: AppColors.primary, + ), + ), + ), + const SizedBox(height: 8), + const Text( + 'Current', + style: TextStyle(color: Color(0xFFB6C2D2), fontSize: 12), + ), + ], + ), + const Padding( + padding: EdgeInsets.symmetric(horizontal: 16), + child: Icon( + Icons.arrow_forward, + color: AppColors.primary, + size: 24, + ), + ), + // New image preview + Column( + children: [ + Container( + width: 100, + height: 100, + decoration: BoxDecoration( + color: AppColors.backgroundSecondary, + borderRadius: BorderRadius.circular(50), + border: Border.all(color: AppColors.primary, width: 2), + ), + child: ClipRRect( + borderRadius: BorderRadius.circular(50), + child: Image.file(image.file, fit: BoxFit.cover), + ), + ), + const SizedBox(height: 8), + const Text( + 'New', + style: TextStyle( + color: AppColors.primary, + fontSize: 12, + fontWeight: FontWeight.w600, + ), + ), + ], + ), + ], + ), + const SizedBox(height: 8), + Center( + child: Text( + community.displayName ?? community.name, + style: const TextStyle( + color: Colors.white, + fontSize: 16, + fontWeight: FontWeight.w600, + ), + ), + ), + Center( + child: Text( + '@${community.handle ?? community.name}', + style: const TextStyle(color: Color(0xFFB6C2D2), fontSize: 14), + ), + ), + const SizedBox(height: 24), + + // Action buttons when image is selected + Row( + children: [ + // Clear button + Expanded( + child: OutlinedButton.icon( + onPressed: _clearSelectedImage, + icon: const Icon(Icons.close), + label: const Text('Clear'), + style: OutlinedButton.styleFrom( + foregroundColor: Colors.white, + side: const BorderSide(color: AppColors.border), + padding: const EdgeInsets.symmetric(vertical: 14), + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(8), + ), + ), + ), + ), + const SizedBox(width: 12), + // Upload button + Expanded( + flex: 2, + child: ElevatedButton.icon( + onPressed: widget.isUploading ? null : _uploadImage, + icon: const Icon(Icons.upload), + label: Text(widget.isUploading ? 'Uploading...' : 'Upload'), + style: ElevatedButton.styleFrom( + backgroundColor: AppColors.primary, + foregroundColor: Colors.white, + padding: const EdgeInsets.symmetric(vertical: 14), + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(8), + ), + disabledBackgroundColor: AppColors.backgroundSecondary, + ), + ), + ), + ], + ), + const SizedBox(height: 12), + // Select different image button + SizedBox( + width: double.infinity, + child: TextButton.icon( + onPressed: _pickAndUploadImage, + icon: const Icon(Icons.photo_library, size: 18), + label: const Text('Select Different Image'), + style: TextButton.styleFrom( + foregroundColor: AppColors.teal, + padding: const EdgeInsets.symmetric(vertical: 12), + ), + ), + ), + ]; + } + + /// The community's existing avatar plus the "pick one" call to action, + /// shown while no new image has been selected. + List _buildCurrentPicture(CommunityView community) { + return [ + const Text( + 'Current Profile Picture', + style: TextStyle( + color: Colors.white, + fontSize: 14, + fontWeight: FontWeight.w500, + ), + ), + const SizedBox(height: 12), + Center( + child: Column( + children: [ + Container( + width: 120, + height: 120, + decoration: BoxDecoration( + color: AppColors.backgroundSecondary, + borderRadius: BorderRadius.circular(60), + border: Border.all(color: AppColors.border, width: 2), + ), + child: CommunityAvatar( + name: community.name, + avatarUrl: community.avatar, + size: 120, + fallbackColor: AppColors.backgroundSecondary, + fallbackIcon: const Icon( + Icons.workspaces_outlined, + size: 48, + color: AppColors.primary, + ), + ), + ), + const SizedBox(height: 8), + Text( + community.displayName ?? community.name, + style: const TextStyle( + color: Colors.white, + fontSize: 16, + fontWeight: FontWeight.w600, + ), + ), + Text( + '@${community.handle ?? community.name}', + style: const TextStyle(color: Color(0xFFB6C2D2), fontSize: 14), + ), + ], + ), + ), + const SizedBox(height: 24), + + // Select image button + SizedBox( + width: double.infinity, + child: ElevatedButton.icon( + onPressed: widget.isUploading ? null : _pickAndUploadImage, + icon: const Icon(Icons.add_photo_alternate), + label: const Text('Select New Picture'), + style: ElevatedButton.styleFrom( + backgroundColor: AppColors.primary, + foregroundColor: Colors.white, + padding: const EdgeInsets.symmetric(vertical: 16), + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(8), + ), + disabledBackgroundColor: AppColors.backgroundSecondary, + ), + ), + ), + ]; + } +} + +/// One selectable community row in the picker list. +class _CommunitySelectTile extends StatelessWidget { + const _CommunitySelectTile({ + required this.community, + required this.isSelected, + required this.onTap, + }); + + final CommunityView community; + final bool isSelected; + final VoidCallback onTap; + + @override + Widget build(BuildContext context) { + return GestureDetector( + onTap: onTap, + child: Container( + margin: const EdgeInsets.only(bottom: 8), + padding: const EdgeInsets.all(12), + decoration: BoxDecoration( + color: AppColors.backgroundSecondary, + borderRadius: BorderRadius.circular(8), + border: Border.all( + color: isSelected ? AppColors.primary : AppColors.border, + width: isSelected ? 2 : 1, + ), + ), + child: Row( + children: [ + CommunityAvatar( + name: community.name, + avatarUrl: community.avatar, + size: 40, + fallbackColor: AppColors.background, + fallbackIcon: const Icon( + Icons.workspaces_outlined, + size: 20, + color: AppColors.primary, + ), + ), + const SizedBox(width: 12), + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + community.displayName ?? community.name, + style: const TextStyle( + color: Colors.white, + fontWeight: FontWeight.w500, + ), + ), + Text( + '@${community.handle ?? community.name}', + style: const TextStyle( + color: Color(0xFFB6C2D2), + fontSize: 12, + ), + ), + ], + ), + ), + if (isSelected) + const Icon( + Icons.check_circle, + color: AppColors.primary, + size: 24, + ), + ], + ), + ), + ); + } +} diff --git a/lib/screens/home/create_community_form.dart b/lib/screens/home/create_community_form.dart new file mode 100644 index 0000000..c216641 --- /dev/null +++ b/lib/screens/home/create_community_form.dart @@ -0,0 +1,377 @@ +import 'package:flutter/material.dart'; + +import '../../constants/app_colors.dart'; +import '../../models/community.dart'; +import '../../utils/community_name_validator.dart'; +import '../../widgets/admin_text_field.dart'; + +/// The admin panel's "Create Community" page body. +/// +/// This widget is built only while the panel is on the create page, so its +/// State is destroyed on every trip back to the menu. It therefore owns +/// exactly one thing — the name error, which is meaningless once the fields +/// it annotates are gone. +/// +/// Everything that must survive that trip is passed in by the panel shell: +/// the draft (three controllers), the receipt list, the in-flight flag, and +/// the submit action itself. A create request outlives this page, so the +/// page must not be the one awaiting it. +class CreateCommunityForm extends StatefulWidget { + const CreateCommunityForm({ + required this.nameController, + required this.displayNameController, + required this.descriptionController, + required this.createdCommunities, + required this.isSubmitting, + required this.onSubmit, + super.key, + }); + + /// Community name (the DNS slug). Owned and disposed by the shell. + final TextEditingController nameController; + + /// Human-readable name. Owned and disposed by the shell. + final TextEditingController displayNameController; + + /// Community description. Owned and disposed by the shell. + final TextEditingController descriptionController; + + /// Communities created so far, newest last. Owned by the shell so the + /// receipts survive a trip to the menu. + final List createdCommunities; + + /// Whether the shell has a create request in flight. Owned there so the + /// button stays disabled even if the admin leaves and comes back. + final bool isSubmitting; + + /// Asks the shell to create a community from the current draft. Called + /// only after [CommunityNameValidator] has accepted the name. + final VoidCallback onSubmit; + + @override + State createState() => _CreateCommunityFormState(); +} + +class _CreateCommunityFormState extends State { + String? _nameError; + + TextEditingController get _nameController => widget.nameController; + TextEditingController get _displayNameController => + widget.displayNameController; + TextEditingController get _descriptionController => + widget.descriptionController; + + /// The controllers this State currently has [_onTextChanged] attached to, + /// in the order the add/remove helpers walk them. + List get _controllers => [ + _nameController, + _displayNameController, + _descriptionController, + ]; + + bool get _isFormValid { + return _nameController.text.trim().isNotEmpty && + _displayNameController.text.trim().isNotEmpty && + _descriptionController.text.trim().isNotEmpty; + } + + /// Preview of the handle the backend will mint from the name field. + /// + /// Lowercased through the same normalizer the validator and the create + /// request use, so the preview can never promise a handle that differs + /// from what is actually sent. + String get _handlePreview { + final name = CommunityNameValidator.normalize(_nameController.text); + if (name.isEmpty) { + return '@c-{name}.coves.social'; + } + return '@c-$name.coves.social'; + } + + // LISTENING IS NOT OWNING. + // + // The three controllers belong to the panel shell, which created them and + // will dispose them. This State attaches to them for exactly as long as it + // is alive and detaches on the way out - so it never disposes one, but it + // does add and remove the listener on every mount/unmount cycle. The + // asymmetry is the point: the shell outlives many of these States, and a + // listener left behind by a torn-down form would call setState on a dead + // State the next time the admin typed. + + @override + void initState() { + super.initState(); + _attachListener(_controllers); + } + + @override + void didUpdateWidget(CreateCommunityForm oldWidget) { + super.didUpdateWidget(oldWidget); + // The shell holds these controllers in final fields, so in practice they + // never change identity. Handled anyway: a swapped-in controller with a + // stale listener on the old one is the classic controller-prop bug. + final previous = [ + oldWidget.nameController, + oldWidget.displayNameController, + oldWidget.descriptionController, + ]; + final current = _controllers; + for (var i = 0; i < current.length; i++) { + if (!identical(previous[i], current[i])) { + previous[i].removeListener(_onTextChanged); + current[i].addListener(_onTextChanged); + } + } + } + + @override + void dispose() { + // Detach only. Disposal is the shell's job - see the note above. + for (final controller in _controllers) { + controller.removeListener(_onTextChanged); + } + super.dispose(); + } + + /// Attaches ONE shared [_onTextChanged] across every field - see that + /// method for why a per-field listener would be wrong. + void _attachListener(List controllers) { + for (final controller in controllers) { + controller.addListener(_onTextChanged); + } + } + + /// Rebuilds on every keystroke in ANY of the three fields, so the submit + /// button's enabled state and the handle preview stay live. + /// + /// Clearing the name error here is deliberately not scoped to the name + /// field: typing anywhere in the form dismisses it. Test-pinned - one + /// shared listener is the mechanism, so do not split this per field. + void _onTextChanged() { + if (_nameError != null) { + setState(() { + _nameError = null; + }); + } else { + setState(() {}); + } + } + + /// Runs the pure validator and projects its answer onto [_nameError]. + bool _validateName() { + final error = CommunityNameValidator.validate(_nameController.text); + setState(() => _nameError = error); + return error == null; + } + + /// Validates, then hands the request to the shell. + /// + /// Deliberately does NOT run the request itself. This State is destroyed + /// the moment the admin taps Back, and a POST that lands afterwards still + /// has to record its receipt, clear the draft and report itself. The shell + /// outlives the page, so it owns the request and its outcome; the only + /// thing left here is the part that is meaningless once the form is gone — + /// the field-level error. + void _submit() { + if (!_isFormValid || widget.isSubmitting) { + return; + } + + if (!_validateName()) { + return; + } + + widget.onSubmit(); + } + + @override + Widget build(BuildContext context) { + return SingleChildScrollView( + padding: const EdgeInsets.all(16), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + const Text( + 'Create Community', + style: TextStyle( + fontSize: 24, + color: Colors.white, + fontWeight: FontWeight.bold, + ), + ), + const SizedBox(height: 8), + const Text( + 'Create a new community for Coves users', + style: TextStyle(fontSize: 14, color: Color(0xFFB6C2D2)), + ), + const SizedBox(height: 24), + + // Name field (DNS-valid slug) + AdminTextField( + controller: _nameController, + label: 'Name (unique identifier)', + hint: 'worldnews', + helperText: 'DNS-valid, lowercase, no spaces', + errorText: _nameError, + ), + const SizedBox(height: 16), + + _HandlePreview(handle: _handlePreview), + const SizedBox(height: 16), + + // Display Name field + AdminTextField( + controller: _displayNameController, + label: 'Display Name', + hint: 'World News', + helperText: 'Human-readable name shown in the UI', + ), + const SizedBox(height: 16), + + // Description field + AdminTextField( + controller: _descriptionController, + label: 'Description', + hint: 'Global news and current events from around the world', + maxLines: 3, + ), + const SizedBox(height: 24), + + // Create button + SizedBox( + width: double.infinity, + child: ElevatedButton( + onPressed: + _isFormValid && !widget.isSubmitting ? _submit : null, + style: ElevatedButton.styleFrom( + backgroundColor: AppColors.primary, + foregroundColor: Colors.white, + padding: const EdgeInsets.symmetric(vertical: 16), + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(8), + ), + disabledBackgroundColor: AppColors.backgroundSecondary, + ), + child: widget.isSubmitting + ? const SizedBox( + height: 20, + width: 20, + child: CircularProgressIndicator( + strokeWidth: 2, + valueColor: AlwaysStoppedAnimation(Colors.white), + ), + ) + : const Text( + 'Create Community', + style: TextStyle( + fontSize: 16, + fontWeight: FontWeight.w600, + ), + ), + ), + ), + + // Created communities list + if (widget.createdCommunities.isNotEmpty) ...[ + const SizedBox(height: 32), + const Text( + 'Created Communities', + style: TextStyle( + fontSize: 18, + color: Colors.white, + fontWeight: FontWeight.bold, + ), + ), + const SizedBox(height: 12), + ...widget.createdCommunities.map( + (community) => _CreatedCommunityTile(community: community), + ), + ], + ], + ), + ); + } +} + +/// Read-only preview of the handle the name field will produce. +class _HandlePreview extends StatelessWidget { + const _HandlePreview({required this.handle}); + + final String handle; + + @override + Widget build(BuildContext context) { + return Container( + padding: const EdgeInsets.all(12), + decoration: BoxDecoration( + color: AppColors.backgroundSecondary, + borderRadius: BorderRadius.circular(8), + border: Border.all(color: AppColors.border), + ), + child: Row( + children: [ + const Icon(Icons.link, color: AppColors.primary, size: 20), + const SizedBox(width: 8), + Expanded( + child: Text( + handle, + style: const TextStyle( + color: AppColors.primary, + fontFamily: 'monospace', + ), + ), + ), + ], + ), + ); + } +} + +/// One row of the "Created Communities" receipt list. +class _CreatedCommunityTile extends StatelessWidget { + const _CreatedCommunityTile({required this.community}); + + final CreateCommunityResponse community; + + @override + Widget build(BuildContext context) { + return Container( + margin: const EdgeInsets.only(bottom: 8), + padding: const EdgeInsets.all(12), + decoration: BoxDecoration( + color: AppColors.backgroundSecondary, + borderRadius: BorderRadius.circular(8), + border: Border.all(color: Colors.green.withValues(alpha: 0.3)), + ), + child: Row( + children: [ + const Icon(Icons.check_circle, color: Colors.green, size: 20), + const SizedBox(width: 12), + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + community.handle, + style: const TextStyle( + color: Colors.white, + fontWeight: FontWeight.w500, + ), + ), + Text( + community.did, + style: const TextStyle( + color: Color(0xFFB6C2D2), + fontSize: 12, + fontFamily: 'monospace', + ), + overflow: TextOverflow.ellipsis, + ), + ], + ), + ), + ], + ), + ); + } +} diff --git a/lib/screens/home/post_detail_loader.dart b/lib/screens/home/post_detail_loader.dart index c508ade..ea0a64a 100644 --- a/lib/screens/home/post_detail_loader.dart +++ b/lib/screens/home/post_detail_loader.dart @@ -9,6 +9,7 @@ import '../../providers/auth_provider.dart'; import '../../providers/vote_provider.dart'; import '../../services/api_exceptions.dart'; import '../../services/coves_api_service.dart'; +import '../../services/viewer_state_hydrator.dart'; import '../../utils/error_messages.dart'; import '../../widgets/loading_error_states.dart'; import 'post_detail_screen.dart'; @@ -149,22 +150,25 @@ class _PostDetailLoaderState extends State { /// /// The response is fresh from getPost, so applying it honors /// [VoteProvider.applyServerVoteState]'s fresh-snapshots-only contract. - /// Skipped when signed out; VoteProvider is read only behind that guard - /// so provider-less widget trees (tests) never look it up. + /// + /// Skipped when signed out, and the [VoteProvider] lookup stays behind + /// that guard so provider-less widget trees (tests, and any screen that + /// only ever renders anonymously) never look it up. The hydrator gates on + /// auth too; this outer gate is about the lookup, not the work, so both + /// stay. void _applyViewerVoteState(PostGetResult result) { if (result is! PostGetSuccess) { return; } - if (!context.read().isAuthenticated) { + final authProvider = context.read(); + if (!authProvider.isAuthenticated) { return; } - final viewer = result.post.viewer; - context.read().applyServerVoteState( - postUri: result.post.uri, - voteDirection: viewer?.vote, - voteUri: viewer?.voteUri, - ); + ViewerStateHydrator( + authProvider: authProvider, + voteProvider: context.read(), + ).hydratePost(result.post); } /// Navigate away: pop if possible, otherwise fall back to the feed diff --git a/lib/services/comments_provider_cache.dart b/lib/services/comments_provider_cache.dart index e084c1c..f7a1cb3 100644 --- a/lib/services/comments_provider_cache.dart +++ b/lib/services/comments_provider_cache.dart @@ -6,6 +6,7 @@ import '../providers/comments_provider.dart'; import '../providers/vote_provider.dart'; import 'comment_service.dart'; import 'coves_api_service.dart'; +import 'viewer_state_hydrator.dart'; /// Comments Provider Cache /// @@ -31,11 +32,13 @@ class CommentsProviderCache { required VoteProvider voteProvider, required CommentService commentService, required CovesApiService apiService, + ViewerStateHydrator? hydrator, this.maxSize = 15, }) : _authProvider = authProvider, _voteProvider = voteProvider, _commentService = commentService, - _apiService = apiService { + _apiService = apiService, + _hydrator = hydrator { _wasAuthenticated = _authProvider.isAuthenticated; _authProvider.addListener(_onAuthChanged); } @@ -45,6 +48,12 @@ class CommentsProviderCache { final CommentService _commentService; final CovesApiService _apiService; + /// Threaded into every [CommentsProvider] this cache builds — these + /// providers are created here rather than by the DI list, so this is the + /// only place the app-wide hydrator can reach them. Null lets each + /// provider fall back to one built from [_voteProvider]. + final ViewerStateHydrator? _hydrator; + /// Maximum number of providers to cache final int maxSize; @@ -134,6 +143,7 @@ class CommentsProviderCache { commentService: _commentService, postUri: postUri, postCid: postCid, + hydrator: _hydrator, ); _cache[postUri] = provider; diff --git a/lib/services/viewer_state_hydrator.dart b/lib/services/viewer_state_hydrator.dart new file mode 100644 index 0000000..049446f --- /dev/null +++ b/lib/services/viewer_state_hydrator.dart @@ -0,0 +1,328 @@ +import 'package:flutter/foundation.dart'; + +import '../models/comment.dart'; +import '../models/community.dart'; +import '../models/post.dart'; +import '../providers/auth_provider.dart'; +import '../providers/community_subscription_provider.dart'; +import '../providers/vote_provider.dart'; + +/// One home for the viewer-state seeding every fetch path used to hand-roll. +/// +/// A Coves response carries the signed-in user's own state alongside the +/// content: `post.viewer.vote` and `community.viewer.subscribed`. Eight call +/// sites used to walk that state into [VoteProvider] and +/// [CommunitySubscriptionProvider] themselves, each with its own traversal +/// and its own copy of the signed-in gate. Only the four provider-layer +/// sites carried a second gate: their vote provider was nullable. The four +/// widget sites resolved theirs with `context.read`, which throws rather +/// than returning null, so a wired-provider check never existed there. +/// +/// This service owns three things: **the gates**, the **per-shape +/// traversal**, and — the one that is easy to miss — the **null-handling +/// policy for `subscribed`**, which is deliberately NOT uniform. Compare +/// [hydrateCommunityListSubscriptions], which coerces a null to false, with +/// [hydrateCommunitySubscription] and the subscription half of +/// [hydrateFeed], which skip. It deliberately does NOT own +/// +/// * *which* items a caller hands over — the feed provider passes the raw +/// response while the pagination controllers pass only deduplicated new +/// items, and that difference is observable on cursor drift, +/// * *when* the caller notifies its listeners, or +/// * what a hydration *throw* does to the page that triggered it. The +/// callers genuinely disagree — some discard the page that just landed, +/// some keep it and report, one lets the error propagate untouched — and +/// each says which at its own call site. Not summarised here: an +/// enumeration this far from the code is how a doc goes stale. +/// +/// Those stay with the callers on purpose. The method names below encode +/// the remaining semantic differences so they cannot be quietly "unified" +/// away. +/// +/// Vote contract: [VoteProvider.applyServerVoteState] is the only +/// server-side write path, and it accepts ONLY snapshots the response being +/// processed actually delivered — never a cached or merge-preserved copy. A +/// null vote direction is still applied: that is how a vote removed on +/// another device gets cleared here, so no vote method below guards against +/// one. Subscriptions are the opposite — a null `subscribed` IS guarded, +/// and, as above, not identically by every method. +class ViewerStateHydrator { + ViewerStateHydrator({ + required AuthProvider authProvider, + VoteProvider? voteProvider, + CommunitySubscriptionProvider? subscriptionProvider, + }) : _authProvider = authProvider, + _voteProvider = voteProvider, + _subscriptionProvider = subscriptionProvider; + + final AuthProvider _authProvider; + + /// Nullable because several construction sites are wired without votes + /// (defensively, or because the surface has none). A null provider makes + /// every vote method a no-op. + final VoteProvider? _voteProvider; + + /// Nullable for the same reason, plus the surfaces that only ever hydrate + /// votes. + final CommunitySubscriptionProvider? _subscriptionProvider; + + bool _warnedMissingVotes = false; + bool _warnedMissingSubscriptions = false; + + /// A copy of this hydrator bound to [authProvider], keeping the same + /// collaborators. + /// + /// UserProfileProvider swaps its AuthProvider at runtime. Before the auth + /// gate moved in here it was read at CALL time, so a swap took effect + /// immediately; a hydrator captured at construction would instead keep + /// gating on the old instance forever. Rebinding through this method + /// restores the original behaviour without exposing the collaborators. + ViewerStateHydrator withAuthProvider(AuthProvider authProvider) { + return ViewerStateHydrator( + authProvider: authProvider, + voteProvider: _voteProvider, + subscriptionProvider: _subscriptionProvider, + ); + } + + /// The vote provider to write through, or null when this hydrator must + /// not touch votes. + /// + /// The two reasons for null are not equally innocent. Signed out is a + /// STATE — nothing to hydrate, stay quiet. A missing provider while + /// signed in is a WIRING BUG, and a silent one: every vote site becomes a + /// permanent no-op, so hearts never light and nothing anywhere complains. + /// It is reported once per hydrator, in debug builds only, so release + /// behaviour stays byte-identical. + /// + /// Deliberately not an `assert`: the characterization net pins a + /// fully-unwired hydrator as a silent no-op, so a throwing assert would + /// fail those tests rather than surface a real defect. + VoteProvider? get _votes { + if (!_authProvider.isAuthenticated) { + return null; + } + final provider = _voteProvider; + if (provider == null) { + assert(() { + if (!_warnedMissingVotes) { + _warnedMissingVotes = true; + debugPrint( + '⚠️ ViewerStateHydrator: no VoteProvider wired - vote state ' + 'from server responses is being silently discarded for this ' + 'surface', + ); + } + return true; + }(), 'debug-only diagnostic'); + } + return provider; + } + + /// The subscription provider to write through, or null when this hydrator + /// must not touch subscriptions. + /// + /// Same reasoning as [_votes]: silent when signed out, one debug-only + /// warning when signed in without a provider. + CommunitySubscriptionProvider? get _subscriptions { + if (!_authProvider.isAuthenticated) { + return null; + } + final provider = _subscriptionProvider; + if (provider == null) { + assert(() { + if (!_warnedMissingSubscriptions) { + _warnedMissingSubscriptions = true; + debugPrint( + '⚠️ ViewerStateHydrator: no CommunitySubscriptionProvider wired ' + '- subscription state from server responses is being silently ' + 'discarded for this surface', + ); + } + return true; + }(), 'debug-only diagnostic'); + } + return provider; + } + + /// Votes *and* community subscriptions for a page of feed items. + /// + /// Serves site 1 ([MultiFeedProvider]'s discover/for-you fetch) and site 2 + /// (the community feed screen's page hook). + /// + /// Which items reach this method is the caller's call and is NOT the same + /// everywhere: site 1 passes the raw response feed, site 2 passes the + /// pagination controller's deduplicated new items. That divergence (D3) is + /// load bearing on cursor drift and must stay with the callers. + void hydrateFeed(Iterable feed) { + _hydrateFeedVotes(feed); + _hydrateFeedSubscriptions(feed); + } + + /// Votes only for a page of feed items, leaving community subscriptions + /// untouched even though `post.community.viewer.subscribed` is right there + /// in the same payload. + /// + /// Serves site 3 (profile posts). This method exists to preserve + /// divergence D1: profile posts have never seeded subscriptions. Keeping + /// it separate from [hydrateFeed] makes that a visible decision instead of + /// an accident of which provider happened to be wired in. + void hydrateFeedVotesOnly(Iterable feed) => + _hydrateFeedVotes(feed); + + /// The vote snapshot on a single cold-loaded post. + /// + /// Serves site 6 ([PostDetailLoader]). That caller resolves its providers + /// *inside* its own auth gate so provider-less widget trees never look + /// them up; the gate here is a second, independent check and does not + /// replace it. + void hydratePost(PostView post) { + final voteProvider = _votes; + if (voteProvider == null) { + return; + } + _applyPostVote(voteProvider, post); + } + + /// Vote snapshots for a flat list of comments — no nested replies. + /// + /// Serves site 4 (profile comments, which the API returns flat). Use + /// [hydrateCommentTree] for threaded responses. + void hydrateComments(Iterable comments) { + final voteProvider = _votes; + if (voteProvider == null) { + return; + } + for (final comment in comments) { + _applyCommentVote(voteProvider, comment); + } + } + + /// Vote snapshots for a comment tree, recursing through every reply at + /// every depth. + /// + /// Serves site 5 ([CommentsProvider]) for both the thread load and the + /// load-more-replies subtree. + /// + /// Callers must pass ONLY the nodes the response delivered. The subtree + /// merge preserves earlier-hydrated branches whose snapshots are old; + /// re-applying those could roll back a vote the server has since confirmed + /// through another surface. + void hydrateCommentTree(Iterable nodes) { + final voteProvider = _votes; + if (voteProvider == null) { + return; + } + for (final node in nodes) { + _applyThreadCommentVote(voteProvider, node); + } + } + + /// Subscription snapshots for a list of communities. + /// + /// Serves site 7 (the communities discovery screen). Preserves the list + /// half of divergence D2: a community whose `viewer` is null is skipped + /// entirely, but a *present* viewer with a null `subscribed` is coerced to + /// false. [hydrateCommunitySubscription] treats that same input the + /// opposite way, on purpose. + void hydrateCommunityListSubscriptions( + Iterable communities, + ) { + final subscriptionProvider = _subscriptions; + if (subscriptionProvider == null) { + return; + } + for (final community in communities) { + final viewer = community.viewer; + if (viewer == null) { + continue; + } + subscriptionProvider.setInitialSubscriptionState( + communityDid: community.did, + isSubscribed: viewer.subscribed ?? false, + ); + } + } + + /// The subscription snapshot on a single community. + /// + /// Serves site 8 (the community feed screen's header load). Preserves the + /// other half of divergence D2: this site skips whenever `subscribed` is + /// null rather than coercing it to false, so a snapshot that says nothing + /// leaves known state alone. + void hydrateCommunitySubscription(CommunityView community) { + final subscriptionProvider = _subscriptions; + if (subscriptionProvider == null) { + return; + } + final subscribed = community.viewer?.subscribed; + if (subscribed == null) { + return; + } + subscriptionProvider.setInitialSubscriptionState( + communityDid: community.did, + isSubscribed: subscribed, + ); + } + + void _hydrateFeedVotes(Iterable feed) { + final voteProvider = _votes; + if (voteProvider == null) { + return; + } + for (final feedItem in feed) { + _applyPostVote(voteProvider, feedItem.post); + } + } + + void _hydrateFeedSubscriptions(Iterable feed) { + final subscriptionProvider = _subscriptions; + if (subscriptionProvider == null) { + return; + } + for (final feedItem in feed) { + final community = feedItem.post.community; + final subscribed = community.viewer?.subscribed; + if (subscribed == null) { + continue; + } + subscriptionProvider.setInitialSubscriptionState( + communityDid: community.did, + isSubscribed: subscribed, + ); + } + } + + void _applyPostVote(VoteProvider voteProvider, PostView post) { + final viewer = post.viewer; + voteProvider.applyServerVoteState( + postUri: post.uri, + voteDirection: viewer?.vote, + voteUri: viewer?.voteUri, + ); + } + + void _applyCommentVote(VoteProvider voteProvider, CommentView comment) { + final viewer = comment.viewer; + voteProvider.applyServerVoteState( + postUri: comment.uri, + voteDirection: viewer?.vote, + voteUri: viewer?.voteUri, + ); + } + + void _applyThreadCommentVote( + VoteProvider voteProvider, + ThreadViewComment node, + ) { + _applyCommentVote(voteProvider, node.comment); + + final replies = node.replies; + if (replies == null) { + return; + } + for (final reply in replies) { + _applyThreadCommentVote(voteProvider, reply); + } + } +} diff --git a/lib/utils/community_name_validator.dart b/lib/utils/community_name_validator.dart new file mode 100644 index 0000000..dab30a9 --- /dev/null +++ b/lib/utils/community_name_validator.dart @@ -0,0 +1,65 @@ +/// Validation for a new community's name — the DNS-style slug that becomes +/// the community's handle. +/// +/// Pure: no widgets, no state, no `setState`. It answers "is this name +/// acceptable, and if not why", and the caller decides what to do with the +/// answer. +library; + +/// Validates and normalizes a community name. +abstract final class CommunityNameValidator { + /// Longest accepted name, in characters. + /// + /// A DNS label may not exceed 63 octets, and the name becomes the + /// `c-` portion of the community handle. + static const int maxLength = 63; + + /// DNS-valid community names: lowercase alphanumerics, interior hyphens. + /// + /// Anchored, and the optional middle group forbids a leading or trailing + /// hyphen while still accepting a single-character name. + static final RegExp _dnsNameRegex = RegExp( + r'^[a-z0-9]([a-z0-9-]*[a-z0-9])?$', + ); + + /// The stored and transmitted form of [rawName]: trimmed, then lowercased. + /// + /// The single definition of the transformation [validate] applies, so + /// validation, the handle preview and the create request cannot drift + /// apart. + static String normalize(String rawName) => rawName.trim().toLowerCase(); + + /// Returns null when [rawName] is acceptable, or the user-facing error + /// message explaining why it is not. + /// + /// The name is [normalize]d — trimmed AND LOWERCASED — before it is + /// tested, so an uppercase name such as `MyCommunity` is ACCEPTED here and + /// stored as `mycommunity`, despite the charset message promising + /// "lowercase letters". That reads like a bug and is not: the create + /// request normalizes the same way, so what is sent always matches what + /// was validated. Rejecting uppercase instead would break the happy path. + /// Test-pinned; if the product wants uppercase rejected, change the + /// message and the test together. + /// + /// The empty-name branch is currently unreachable from the admin panel — + /// its submit button is disabled while any field is blank, and + /// [normalize] cannot empty a non-empty string — but it is the contract + /// for any other caller and is kept deliberately. + static String? validate(String rawName) { + final name = normalize(rawName); + + if (name.isEmpty) { + return 'Name is required'; + } + + if (name.length > maxLength) { + return 'Name must be $maxLength characters or less'; + } + + if (!_dnsNameRegex.hasMatch(name)) { + return 'Name must be lowercase letters, numbers, and hyphens only'; + } + + return null; + } +} diff --git a/lib/widgets/admin_text_field.dart b/lib/widgets/admin_text_field.dart new file mode 100644 index 0000000..a5fd99d --- /dev/null +++ b/lib/widgets/admin_text_field.dart @@ -0,0 +1,91 @@ +import 'package:flutter/material.dart'; + +import '../constants/app_colors.dart'; + +/// A labelled dark-theme text field for the admin forms. +/// +/// Deliberately dumb: it renders a label, a [TextField] and either a helper +/// or an error line, and owns nothing. The [controller] is created, listened +/// to and disposed by the form that supplies it — the admin create form +/// attaches ONE shared listener across all of its fields, and a listener +/// added here per field would quietly break that. +class AdminTextField extends StatelessWidget { + const AdminTextField({ + required this.controller, + required this.label, + required this.hint, + this.helperText, + this.errorText, + this.maxLines = 1, + super.key, + }); + + /// Owned by the caller: never listened to or disposed here. + final TextEditingController controller; + + /// Field label rendered above the input. + final String label; + + /// Placeholder shown while the field is empty. + final String hint; + + /// Guidance shown below the input. Suppressed while [errorText] is set, + /// so the two never stack. + final String? helperText; + + /// Validation message. Non-null also turns the borders red. + final String? errorText; + + /// Number of visible lines; more than one makes the field multiline. + final int maxLines; + + @override + Widget build(BuildContext context) { + final hasError = errorText != null; + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + label, + style: const TextStyle( + color: Colors.white, + fontSize: 14, + fontWeight: FontWeight.w500, + ), + ), + const SizedBox(height: 8), + TextField( + controller: controller, + maxLines: maxLines, + style: const TextStyle(color: Colors.white), + decoration: InputDecoration( + hintText: hint, + hintStyle: TextStyle(color: Colors.white.withValues(alpha: 0.4)), + helperText: hasError ? null : helperText, + helperStyle: const TextStyle(color: Color(0xFFB6C2D2)), + errorText: errorText, + errorStyle: const TextStyle(color: Colors.red), + filled: true, + fillColor: AppColors.backgroundSecondary, + border: OutlineInputBorder( + borderRadius: BorderRadius.circular(8), + borderSide: const BorderSide(color: AppColors.border), + ), + enabledBorder: OutlineInputBorder( + borderRadius: BorderRadius.circular(8), + borderSide: BorderSide( + color: hasError ? Colors.red : AppColors.border, + ), + ), + focusedBorder: OutlineInputBorder( + borderRadius: BorderRadius.circular(8), + borderSide: BorderSide( + color: hasError ? Colors.red : AppColors.primary, + ), + ), + ), + ), + ], + ); + } +} diff --git a/test/providers/comment_thread_tree_characterization_test.dart b/test/providers/comment_thread_tree_characterization_test.dart new file mode 100644 index 0000000..284e760 --- /dev/null +++ b/test/providers/comment_thread_tree_characterization_test.dart @@ -0,0 +1,883 @@ +// Characterization net for the comment-tree algebra now extracted into +// lib/models/comment_thread_tree.dart (merge, node replacement, lookup, +// membership, and the subtree-construction decision). +// +// Every test drives that algebra through CommentsProvider's public surface +// (loadComments / loadMoreReplies / comments) with a faked CovesApiService, +// matching comments_provider_test.dart's idioms. It was written this way +// while the algebra was still private to the provider, and it stays that way +// on purpose: asserting through the provider proves the extracted type is +// wired up correctly, not merely correct in isolation. Direct unit tests of +// the tree type are complementary, not a replacement. +// +// The merge has FOUR branches and they disagree with each other in ways that +// look like bugs and are not: +// +// * fresh replies empty + existing replies present -> keep the existing +// branch AND existing.hasMore AND existing.repliesCursor (T2) +// * fresh replies empty + existing replies empty -> return fresh +// VERBATIM, which DROPS existing.repliesCursor (T3) +// * recursive branch -> repliesCursor from existing but hasMore from +// FRESH, the opposite of the truncation branch above (T7) +// * leftover existing children are appended when fresh.hasMore is true and +// DROPPED when it is false (T5/T6) +// +// T2/T3/T7 are only observable at NESTED depth: for the node the request was +// anchored at, the caller's outer copyWith overwrites hasMore and +// repliesCursor from the response cursor, masking whatever the merge chose. + +import 'dart:async'; + +import 'package:coves_flutter/models/comment.dart'; +import 'package:coves_flutter/models/post.dart'; +import 'package:coves_flutter/providers/comments_provider.dart'; +import 'package:coves_flutter/services/comment_service.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; + +import '../test_helpers/test_mocks.dart'; + +const String commentBase = 'at://did:plc:author/social.coves.community.comment'; +const String rootUri = '$commentBase/root'; +const String otherRootUri = '$commentBase/otherroot'; +const String childUri = '$commentBase/child'; +const String siblingUri = '$commentBase/sibling'; +const String grandchildUri = '$commentBase/grandchild'; +const String greatGrandUri = '$commentBase/greatgrand'; +const String orphanUri = '$commentBase/orphan'; +const String replyAUri = '$commentBase/replya'; +const String replyBUri = '$commentBase/replyb'; + +/// The rkey the provider derives from an AT-URI: its last path segment. +String rkeyOf(String uri) => uri.split('/').last; + +CommentView buildView(String uri, {String content = 'body', int score = 0}) { + return CommentView( + uri: uri, + cid: 'cid-$uri', + record: 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/social.coves.post.record/123', + cid: 'post-cid', + ), + stats: CommentStats(score: score, upvotes: score), + ); +} + +ThreadViewComment node( + String uri, { + String content = 'body', + int score = 0, + List? replies, + bool hasMore = false, +}) { + return ThreadViewComment( + comment: buildView(uri, content: content, score: score), + replies: replies, + hasMore: hasMore, + ); +} + +List repliesOf(ThreadViewComment? parent) => + (parent?.replies ?? const []) + .map((r) => r.comment.uri) + .toList(); + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + const testPostUri = 'at://did:plc:test/social.coves.post.record/123'; + const testPostCid = 'test-post-cid'; + + late MockAuthProvider mockAuthProvider; + late MockCovesApiService mockApiService; + late CommentsProvider provider; + + /// Queued subtree responses, keyed by the rkey the provider will send. + /// + /// Strictly one queued response per expected fetch, consumed in order: a + /// test that under-enqueues fails loudly rather than silently replaying + /// the previous page. + late Map>> subtreeQueues; + + void enqueueSubtree(String uri, CommentsResponse response) { + (subtreeQueues[rkeyOf(uri)] ??= >[]).add( + Future.value(response), + ); + } + + void enqueueSubtreeFuture(String uri, Future response) { + (subtreeQueues[rkeyOf(uri)] ??= >[]).add(response); + } + + /// Answers the top-level thread fetch (the one with no parentRkey). + void stubInitialTree(List comments) { + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).thenAnswer( + (_) async => CommentsResponse(post: {}, comments: comments), + ); + } + + setUp(() { + mockAuthProvider = MockAuthProvider(); + mockApiService = MockCovesApiService(); + subtreeQueues = >>{}; + + when(mockAuthProvider.isAuthenticated).thenReturn(true); + when( + mockAuthProvider.getAccessToken(), + ).thenAnswer((_) async => 'test-token'); + + // One stub for every subtree fetch; the rkey selects the response. + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + parentRkey: argThat(isNotNull, named: 'parentRkey'), + ), + ).thenAnswer((invocation) { + final rkey = invocation.namedArguments[#parentRkey] as String; + final queue = subtreeQueues[rkey]; + if (queue == null || queue.isEmpty) { + return Future.error( + StateError('no subtree response queued for rkey "$rkey"'), + ); + } + return queue.removeAt(0); + }); + + provider = CommentsProvider( + mockAuthProvider, + postUri: testPostUri, + postCid: testPostCid, + apiService: mockApiService, + ); + }); + + tearDown(() { + provider.dispose(); + }); + + /// Gives [uri]'s node a stored repliesCursor the only way production can: + /// by completing one real load-more page against it. + Future seedRepliesCursor( + String uri, { + required List replies, + required String cursor, + }) async { + enqueueSubtree( + uri, + CommentsResponse( + post: {}, + comments: [node(uri, replies: replies)], + cursor: cursor, + ), + ); + await provider.loadMoreReplies(uri); + } + + group('merge algebra', () { + test('T1 fresh wins for node content and stats', () async { + stubInitialTree([ + node(rootUri, content: 'stale', score: 1, replies: [node(childUri)]), + ]); + await provider.loadComments(refresh: true); + + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node( + rootUri, + content: 'fresh', + score: 42, + replies: [node(childUri)], + ), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + + final merged = provider.comments.single; + expect(merged.comment.record?.content, 'fresh'); + expect(merged.comment.stats.score, 42); + }); + + test('T2 truncated fresh node keeps the existing branch, its hasMore ' + 'and its repliesCursor', () async { + stubInitialTree([ + node( + rootUri, + replies: [ + node(childUri, hasMore: true, replies: [node(grandchildUri)]), + ], + ), + ]); + await provider.loadComments(refresh: true); + + await seedRepliesCursor( + childUri, + replies: [node(grandchildUri)], + cursor: 'child-cursor', + ); + + // The ancestor refetch truncates the child at the depth cutoff: no + // replies, and hasMore false so "true" below can only be the + // existing node's. + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, replies: [node(childUri)]), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + + final child = provider.comments.single.replies!.single; + expect(repliesOf(child), [grandchildUri]); + expect(child.hasMore, isTrue); + expect(child.repliesCursor, 'child-cursor'); + }); + + test('T3 when BOTH reply lists are empty the fresh node is returned ' + 'verbatim, dropping the existing repliesCursor', () async { + stubInitialTree([ + node(rootUri, replies: [node(childUri, hasMore: true)]), + ]); + await provider.loadComments(refresh: true); + + // The child ends up with a cursor but still no loaded replies. + await seedRepliesCursor( + childUri, + replies: const [], + cursor: 'child-cursor', + ); + expect(provider.comments.single.replies!.single.repliesCursor, + 'child-cursor'); + + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, replies: [node(childUri)]), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + + final child = provider.comments.single.replies!.single; + // Same input shape as T2 except the existing branch was empty - and + // the answer for the cursor is the opposite one. + expect(child.repliesCursor, isNull); + expect(child.hasMore, isFalse); + }); + + test('T4 children present in both are merged recursively so ' + 'grandchildren survive', () async { + stubInitialTree([ + node( + rootUri, + replies: [ + node( + childUri, + replies: [ + node(grandchildUri, replies: [node(greatGrandUri)]), + ], + ), + ], + ), + ]); + await provider.loadComments(refresh: true); + + // Fresh response truncates two levels down: the grandchild comes back + // with no replies at all. + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node( + rootUri, + replies: [ + node(childUri, replies: [node(grandchildUri)]), + ], + ), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + + final grandchild = + provider.comments.single.replies!.single.replies!.single; + expect(grandchild.comment.uri, grandchildUri); + expect(repliesOf(grandchild), [greatGrandUri]); + }); + + test('T5 fresh.hasMore true appends leftover existing children after ' + 'the fresh ordering', () async { + stubInitialTree([ + node(rootUri, replies: [node(replyAUri), node(replyBUri)]), + ]); + await provider.loadComments(refresh: true); + + // A sibling-truncated page: only reply B, but more are promised. + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, hasMore: true, replies: [node(replyBUri)]), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + + expect(repliesOf(provider.comments.single), [replyBUri, replyAUri]); + }); + + test('T6 fresh.hasMore false DROPS leftover existing children', () async { + stubInitialTree([ + node(rootUri, replies: [node(replyAUri), node(replyBUri)]), + ]); + await provider.loadComments(refresh: true); + + // A complete listing: absence now means deletion. + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, replies: [node(replyBUri)]), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + + expect(repliesOf(provider.comments.single), [replyBUri]); + }); + + test('T7 on the recursive branch repliesCursor comes from existing but ' + 'hasMore comes from fresh', () async { + stubInitialTree([ + node( + rootUri, + replies: [ + node(childUri, hasMore: true, replies: [node(grandchildUri)]), + ], + ), + ]); + await provider.loadComments(refresh: true); + + await seedRepliesCursor( + childUri, + replies: [node(grandchildUri)], + cursor: 'child-cursor', + ); + expect(provider.comments.single.replies!.single.hasMore, isTrue); + + // This time the fresh child DOES carry replies, so the recursive + // branch runs instead of the truncation branch of T2. + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node( + rootUri, + replies: [ + node(childUri, replies: [node(grandchildUri)]), + ], + ), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + + final child = provider.comments.single.replies!.single; + // Cursor kept from the existing node... + expect(child.repliesCursor, 'child-cursor'); + // ...but hasMore taken from fresh, discarding the existing true. + // T2 answered the same question the other way round. + expect(child.hasMore, isFalse); + }); + }); + + group('node replacement and lookup', () { + test('T8 a node absent from the top-level tree leaves the list instance ' + 'untouched while still returning the subtree', () async { + stubInitialTree([node(rootUri)]); + await provider.loadComments(refresh: true); + final before = provider.comments; + + enqueueSubtree( + orphanUri, + CommentsResponse( + post: {}, + comments: [ + node(orphanUri, replies: [node(replyAUri)]), + ], + ), + ); + final result = await provider.loadMoreReplies(orphanUri); + + expect(result, isNotNull); + expect(result!.comment.uri, orphanUri); + expect(repliesOf(result), [replyAUri]); + // Same list instance: nothing was merged, and callers detect that + // through identity rather than a flag. + expect(identical(before, provider.comments), isTrue); + }); + + test('T9 replacing a node with an instance already in the tree reports ' + 'no change and hands the same subtree back', () { + // Not reachable through loadMoreReplies - every subtree it builds is + // a fresh copyWith, so the replacement is never an instance already + // in the tree. This pins the model-level identity contract that the + // list-level replace relies on to decide whether anything changed. + final child = node(childUri); + final root = node(rootUri, replies: [child, node(siblingUri)]); + + expect(identical(root.replaceDescendant(child), root), isTrue); + }); + + test('T10 replaces a nested descendant and preserves sibling branches ' + 'by identity', () async { + final sibling = node(siblingUri); + final otherRoot = node(otherRootUri); + stubInitialTree([ + node(rootUri, replies: [node(childUri), sibling]), + otherRoot, + ]); + await provider.loadComments(refresh: true); + + enqueueSubtree( + childUri, + CommentsResponse( + post: {}, + comments: [ + node(childUri, replies: [node(grandchildUri)]), + ], + ), + ); + await provider.loadMoreReplies(childUri); + + final updatedRoot = provider.comments.first; + expect(repliesOf(updatedRoot), [childUri, siblingUri]); + expect(repliesOf(updatedRoot.replies!.first), [grandchildUri]); + // Untouched branches keep reference identity. + expect(identical(updatedRoot.replies![1], sibling), isTrue); + expect(identical(provider.comments[1], otherRoot), isTrue); + }); + + test('T11 the node lookup reaches every depth of the tree', () async { + stubInitialTree([ + node( + rootUri, + replies: [ + node(childUri, replies: [node(grandchildUri, hasMore: true)]), + ], + ), + ]); + await provider.loadComments(refresh: true); + + // Anchored three levels down: found, so it merges in place rather + // than falling through to the "not in tree" path of T8. + enqueueSubtree( + grandchildUri, + CommentsResponse( + post: {}, + comments: [ + node(grandchildUri, replies: [node(greatGrandUri)]), + ], + ), + ); + await provider.loadMoreReplies(grandchildUri); + + final grandchild = + provider.comments.single.replies!.single.replies!.single; + expect(repliesOf(grandchild), [greatGrandUri]); + }); + }); + + group('subtree construction from the response', () { + test('T12 a cursor page appends new direct replies deduplicated by URI ' + 'and takes hasMore/repliesCursor from the response cursor', () async { + stubInitialTree([node(rootUri, hasMore: true)]); + await provider.loadComments(refresh: true); + + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, replies: [node(replyAUri)]), + ], + cursor: 'page-2', + ), + ); + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + // The server re-sends reply A alongside the new reply B. + node(rootUri, replies: [node(replyAUri), node(replyBUri)]), + ], + cursor: 'page-3', + ), + ); + + await provider.loadMoreReplies(rootUri); + await provider.loadMoreReplies(rootUri); + + // The stored cursor was sent back, which is what selects this branch. + verify( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: 'page-2', + parentRkey: argThat(equals('root'), named: 'parentRkey'), + ), + ).called(1); + + final merged = provider.comments.single; + expect(repliesOf(merged), [replyAUri, replyBUri]); + expect(merged.hasMore, isTrue); + expect(merged.repliesCursor, 'page-3'); + }); + + test('T13 a first page with an existing node merges, then takes ' + 'hasMore/repliesCursor from the response cursor', () async { + stubInitialTree([ + node(rootUri, replies: [node(replyAUri)]), + ]); + await provider.loadComments(refresh: true); + + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, hasMore: true, replies: [node(replyBUri)]), + ], + cursor: 'page-2', + ), + ); + await provider.loadMoreReplies(rootUri); + + final merged = provider.comments.single; + // Merged, not replaced: reply A survived as a leftover. + expect(repliesOf(merged), [replyBUri, replyAUri]); + expect(merged.hasMore, isTrue); + expect(merged.repliesCursor, 'page-2'); + }); + + test('T14 a first page with no existing node takes the fresh subtree ' + 'plus the response cursor state', () async { + stubInitialTree([node(rootUri)]); + await provider.loadComments(refresh: true); + + enqueueSubtree( + orphanUri, + CommentsResponse( + post: {}, + comments: [ + node(orphanUri, replies: [node(replyAUri)]), + ], + cursor: 'page-2', + ), + ); + final result = await provider.loadMoreReplies(orphanUri); + + expect(repliesOf(result), [replyAUri]); + expect(result!.hasMore, isTrue); + expect(result.repliesCursor, 'page-2'); + }); + }); + + group('behaviour that must stay with the provider', () { + test('an empty response clears the node pagination state and returns ' + 'null, keeping the loaded replies', () async { + stubInitialTree([node(rootUri, hasMore: true)]); + await provider.loadComments(refresh: true); + + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, replies: [node(replyAUri)]), + ], + cursor: 'page-2', + ), + ); + await provider.loadMoreReplies(rootUri); + expect(provider.comments.single.repliesCursor, 'page-2'); + + enqueueSubtree(rootUri, CommentsResponse(post: {}, comments: const [])); + final result = await provider.loadMoreReplies(rootUri); + + // Returning null is what drives the create-comment retry loop. + expect(result, isNull); + final cleared = provider.comments.single; + expect(cleared.hasMore, isFalse); + expect(cleared.repliesCursor, isNull); + expect(repliesOf(cleared), [replyAUri]); + }); + + test('an empty response is a genuine no-op when the node has neither ' + 'hasMore nor a repliesCursor', () async { + stubInitialTree([node(rootUri)]); + await provider.loadComments(refresh: true); + final before = provider.comments; + + enqueueSubtree(rootUri, CommentsResponse(post: {}, comments: const [])); + final result = await provider.loadMoreReplies(rootUri); + + expect(result, isNull); + expect(identical(before, provider.comments), isTrue); + }); + + test('an empty response is accepted without the anchoring check the ' + 'non-empty path applies', () async { + // The anchoring guard reads response.comments.first, so it can only + // run after the empty-response branch has already returned. An empty + // response therefore clears pagination state no matter which comment + // the server thought it was answering about. + stubInitialTree([node(rootUri, hasMore: true)]); + await provider.loadComments(refresh: true); + + enqueueSubtree(rootUri, CommentsResponse(post: {}, comments: const [])); + expect(await provider.loadMoreReplies(rootUri), isNull); + expect(provider.comments.single.hasMore, isFalse); + + // Contrast: a NON-empty response anchored elsewhere is discarded and + // leaves the tree alone. + stubInitialTree([node(rootUri, hasMore: true)]); + await provider.loadComments(refresh: true); + + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(orphanUri, replies: [node(replyAUri)]), + ], + ), + ); + expect(await provider.loadMoreReplies(rootUri), isNull); + expect(provider.comments.single.hasMore, isTrue); + expect(provider.comments.single.replies, isNull); + }); + + test('the request cursor is captured before the fetch while the ' + 'existing node is looked up after it, so the two can disagree', + () async { + stubInitialTree([ + node(rootUri, replies: [node(childUri, hasMore: true)]), + ]); + await provider.loadComments(refresh: true); + + await seedRepliesCursor( + childUri, + replies: [node(replyAUri)], + cursor: 'child-cursor', + ); + + // Start a cursor page for the child, then strand it mid-flight. + final gate = Completer(); + enqueueSubtreeFuture(childUri, gate.future); + final pending = provider.loadMoreReplies(childUri); + + // The ancestor refetch delivers a complete listing without the child, + // so the child is dropped from the tree while its page is in flight. + enqueueSubtree( + rootUri, + CommentsResponse( + post: {}, + comments: [ + node(rootUri, replies: [node(siblingUri)]), + ], + ), + ); + await provider.loadMoreReplies(rootUri); + expect(repliesOf(provider.comments.single), [siblingUri]); + + gate.complete( + CommentsResponse( + post: {}, + comments: [ + node(childUri, replies: [node(replyBUri)]), + ], + ), + ); + final result = await pending; + + // The cursor WAS sent, so the request believed it was paginating... + verify( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: 'child-cursor', + parentRkey: argThat(equals('child'), named: 'parentRkey'), + ), + ).called(1); + + // ...but by the time the response landed the node was gone, so the + // first-page branch ran: reply A was NOT appended. + expect(repliesOf(result), [replyBUri]); + expect(provider.comments.single.replies!.single.comment.uri, siblingUri); + }); + + test('a subtree whose tree was refreshed mid-flight is discarded whole', + () async { + stubInitialTree([ + node(rootUri, replies: [node(childUri, hasMore: true)]), + ]); + await provider.loadComments(refresh: true); + + final gate = Completer(); + enqueueSubtreeFuture(childUri, gate.future); + final pending = provider.loadMoreReplies(childUri); + + await provider.refreshComments(); + + gate.complete( + CommentsResponse( + post: {}, + comments: [ + node(childUri, replies: [node(replyAUri)]), + ], + ), + ); + + expect(await pending, isNull); + // No state, no error, no merge: the tree is the refreshed one. + expect(provider.comments.single.replies!.single.replies, isNull); + }); + }); + + group('tree membership drives the create-comment verification path', () { + late MockCommentService mockCommentService; + late CommentsProvider creatingProvider; + + const newCommentUri = '$commentBase/created'; + + /// Counts the top-level thread fetches (the ones with no parentRkey). + void expectTopLevelFetches(int count) { + verify( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).called(count); + } + + setUp(() { + mockCommentService = MockCommentService(); + when( + mockCommentService.createComment( + rootUri: anyNamed('rootUri'), + rootCid: anyNamed('rootCid'), + parentUri: anyNamed('parentUri'), + parentCid: anyNamed('parentCid'), + content: anyNamed('content'), + contentFacets: anyNamed('contentFacets'), + ), + ).thenAnswer( + (_) async => const CreateCommentResponse( + uri: newCommentUri, + cid: 'cid-created', + ), + ); + + creatingProvider = CommentsProvider( + mockAuthProvider, + postUri: testPostUri, + postCid: testPostCid, + apiService: mockApiService, + commentService: mockCommentService, + // No backoff: the verification loops run once and stop. + indexingRetryDelays: const [], + ); + }); + + tearDown(() { + creatingProvider.dispose(); + }); + + test('a parent nested deep in the tree counts as present and takes the ' + 'refresh path', () async { + final parent = node(grandchildUri); + stubInitialTree([ + node( + rootUri, + replies: [ + node(childUri, replies: [parent]), + ], + ), + ]); + await creatingProvider.loadComments(refresh: true); + enqueueSubtree( + grandchildUri, + CommentsResponse(post: {}, comments: [node(grandchildUri)]), + ); + + await creatingProvider.createComment( + content: 'hello', + parentComment: parent, + ); + + // Initial load plus the refresh the present-parent path performs. + expectTopLevelFetches(2); + }); + + test('a parent absent from the tree skips the refresh and verifies ' + 'against the returned subtree instead', () async { + stubInitialTree([node(rootUri)]); + await creatingProvider.loadComments(refresh: true); + enqueueSubtree( + orphanUri, + CommentsResponse(post: {}, comments: [node(orphanUri)]), + ); + + await creatingProvider.createComment( + content: 'hello', + parentComment: node(orphanUri), + ); + + // Only the initial load: no refresh, because a refresh could never + // surface a reply below the depth cap. + expectTopLevelFetches(1); + }); + }); +} diff --git a/test/providers/viewer_state_hydration_characterization_test.dart b/test/providers/viewer_state_hydration_characterization_test.dart new file mode 100644 index 0000000..4a47934 --- /dev/null +++ b/test/providers/viewer_state_hydration_characterization_test.dart @@ -0,0 +1,737 @@ +// Characterization net for the viewer-state hydration the provider-layer +// sites perform today (sites 1, 3, 4 and 5 of the eight). +// +// These tests exist to survive the extraction of that logic into a shared +// hydrator, so they assert ONLY through the public surfaces the refactor +// keeps: VoteProvider.isLiked / getVoteState / getAdjustedScore and +// CommunitySubscriptionProvider.isSubscribed. No private helper is named. +// +// The sites are deliberately NOT interchangeable. Where two of them treat +// the same input differently that difference is pinned on purpose - see the +// "divergence" comments. A test here failing after the refactor means a +// behaviour changed, not that the test is stale. +// +// Follows the existing vote-regression files: a real VoteProvider (the +// routing under test lives there) with a hand-rolled VoteService fake, plus +// the shared generated mocks for the API and auth surfaces. + +import 'package:coves_flutter/models/comment.dart'; +import 'package:coves_flutter/models/post.dart'; +import 'package:coves_flutter/models/user_profile.dart'; +import 'package:coves_flutter/providers/comments_provider.dart'; +import 'package:coves_flutter/providers/community_subscription_provider.dart'; +import 'package:coves_flutter/providers/multi_feed_provider.dart'; +import 'package:coves_flutter/providers/user_profile_provider.dart'; +import 'package:coves_flutter/providers/vote_provider.dart'; +import 'package:coves_flutter/services/viewer_state_hydrator.dart'; +import 'package:coves_flutter/services/vote_service.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; + +import '../test_helpers/test_mocks.dart'; + +/// A VoteService that answers locally instead of hitting the network. +class _FakeVoteService implements VoteService { + _FakeVoteService({required this.response}); + + VoteResponse response; + + @override + Future createVote({ + required String postUri, + required String postCid, + String direction = 'up', + }) async { + return response; + } +} + +const String communityDid = 'did:plc:community'; +const String voteUri = 'at://did:plc:me/social.coves.feed.vote/v1'; +const String postUriA = 'at://did:plc:author/social.coves.community.post/p1'; +const String postUriB = 'at://did:plc:author/social.coves.community.post/p2'; + +FeedViewPost buildFeedPost({ + required String uri, + String? vote, + String? voteUri, + CommunityRefViewerState? communityViewer, + int score = 0, +}) { + return FeedViewPost( + post: PostView( + uri: uri, + cid: 'cid-$uri', + rkey: uri.split('/').last, + author: AuthorView(did: 'did:plc:author', handle: 'test.user'), + community: CommunityRef( + did: communityDid, + name: 'testcove', + viewer: communityViewer, + ), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + record: const PostRecord(title: 'Title', content: 'Body'), + stats: PostStats( + upvotes: score, + downvotes: 0, + score: score, + commentCount: 0, + ), + viewer: ViewerState(vote: vote, voteUri: voteUri), + ), + ); +} + +CommentView buildComment({required String uri, String? vote, String? voteUri}) { + return CommentView( + uri: uri, + cid: 'cid-$uri', + record: const CommentRecord(content: 'Test comment 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/social.coves.post.record/123', + cid: 'post-cid', + ), + stats: const CommentStats(score: 5, upvotes: 5), + viewer: CommentViewerState(vote: vote, voteUri: voteUri), + ); +} + +ThreadViewComment buildThreadComment({ + required String uri, + String? vote, + String? voteUri, + List? replies, +}) { + return ThreadViewComment( + comment: buildComment(uri: uri, vote: vote, voteUri: voteUri), + replies: replies, + ); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late MockAuthProvider mockAuthProvider; + late MockCovesApiService mockApiService; + late VoteProvider voteProvider; + + setUp(() { + mockAuthProvider = MockAuthProvider(); + mockApiService = MockCovesApiService(); + + when(mockAuthProvider.isAuthenticated).thenReturn(true); + when(mockAuthProvider.did).thenReturn('did:plc:me'); + when( + mockAuthProvider.getAccessToken(), + ).thenAnswer((_) async => 'test-token'); + + voteProvider = VoteProvider( + voteService: _FakeVoteService( + response: const VoteResponse( + uri: voteUri, + cid: 'bafyvote', + rkey: 'v1', + deleted: false, + ), + ), + authProvider: mockAuthProvider, + ); + }); + + tearDown(() { + voteProvider.dispose(); + }); + + CommunitySubscriptionProvider newSubscriptionProvider() { + final provider = CommunitySubscriptionProvider( + authProvider: mockAuthProvider, + apiService: mockApiService, + ); + addTearDown(provider.dispose); + return provider; + } + + /// Stubs getDiscover to answer [pages] in order, repeating the last. + void stubDiscoverPages(List pages) { + var call = 0; + when( + mockApiService.getDiscover( + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).thenAnswer((_) async { + final page = pages[call < pages.length ? call : pages.length - 1]; + call++; + return page; + }); + } + + /// Stubs getAuthorPosts to answer [pages] in order, repeating the last. + void stubAuthorPostsPages(List pages) { + var call = 0; + when( + mockApiService.getAuthorPosts( + actor: anyNamed('actor'), + filter: anyNamed('filter'), + community: anyNamed('community'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).thenAnswer((_) async { + final page = pages[call < pages.length ? call : pages.length - 1]; + call++; + return page; + }); + } + + void stubActorComments(List comments) { + when( + mockApiService.getActorComments( + actor: anyNamed('actor'), + community: anyNamed('community'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).thenAnswer((_) async => ActorCommentsResponse(comments: comments)); + } + + void stubThreadResponse(List comments) { + when( + mockApiService.getComments( + postUri: anyNamed('postUri'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + depth: anyNamed('depth'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).thenAnswer((_) async => CommentsResponse(post: {}, comments: comments)); + } + + void stubSubtreeResponse(ThreadViewComment node) { + 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: [node])); + } + + MultiFeedProvider newFeedProvider({ + VoteProvider? votes, + CommunitySubscriptionProvider? subscriptions, + }) { + final provider = MultiFeedProvider( + mockAuthProvider, + apiService: mockApiService, + voteProvider: votes, + subscriptionProvider: subscriptions, + ); + addTearDown(provider.dispose); + return provider; + } + + Future newProfileProvider({ + VoteProvider? votes, + ViewerStateHydrator? hydrator, + }) async { + const profileDid = 'did:plc:profileowner'; + final provider = UserProfileProvider( + mockAuthProvider, + apiService: mockApiService, + voteProvider: votes, + hydrator: hydrator, + commentService: MockCommentService(), + ); + addTearDown(provider.dispose); + + when(mockApiService.getProfile(actor: anyNamed('actor'))).thenAnswer( + (_) async => UserProfile(did: profileDid, handle: 'owner.test'), + ); + await provider.loadProfile(profileDid); + return provider; + } + + CommentsProvider newCommentsProvider({ + VoteProvider? votes, + ViewerStateHydrator? hydrator, + }) { + final provider = CommentsProvider( + mockAuthProvider, + postUri: 'at://did:plc:test/social.coves.post.record/123', + postCid: 'test-post-cid', + apiService: mockApiService, + voteProvider: votes, + hydrator: hydrator, + ); + addTearDown(provider.dispose); + return provider; + } + + group('feed hydration - votes and subscriptions (MultiFeedProvider)', () { + test('C1 applies the vote snapshot for every post, including a null ' + 'direction that clears a vote removed elsewhere', () async { + // Adopted from an earlier surface, no local mutation outstanding. + voteProvider.applyServerVoteState( + postUri: postUriB, + voteDirection: 'up', + voteUri: voteUri, + ); + expect(voteProvider.isLiked(postUriB), true); + + stubDiscoverPages([ + TimelineResponse( + feed: [ + buildFeedPost(uri: postUriA, vote: 'up', voteUri: voteUri), + // Vote removed on another device: the null direction is applied, + // never ignored. + buildFeedPost(uri: postUriB), + ], + ), + ]); + + final feed = newFeedProvider(votes: voteProvider); + await feed.loadFeed(FeedType.discover, refresh: true); + + expect(voteProvider.isLiked(postUriA), true); + expect(voteProvider.getVoteState(postUriA)?.uri, voteUri); + expect(voteProvider.isLiked(postUriB), false); + }); + + test('C2 applies the subscription snapshot when ' + 'community.viewer.subscribed is set', () async { + final subscriptions = newSubscriptionProvider(); + stubDiscoverPages([ + TimelineResponse( + feed: [ + buildFeedPost( + uri: postUriA, + communityViewer: CommunityRefViewerState(subscribed: true), + ), + ], + ), + ]); + + final feed = newFeedProvider( + votes: voteProvider, + subscriptions: subscriptions, + ); + await feed.loadFeed(FeedType.discover, refresh: true); + + expect(subscriptions.isSubscribed(communityDid), true); + }); + + test('C3 skips the subscription when community.viewer.subscribed is ' + 'null, leaving known state untouched', () async { + final subscriptions = newSubscriptionProvider() + ..setInitialSubscriptionState( + communityDid: communityDid, + isSubscribed: true, + ); + + stubDiscoverPages([ + TimelineResponse( + feed: [ + // viewer present but subscribed omitted... + buildFeedPost( + uri: postUriA, + communityViewer: CommunityRefViewerState(), + ), + // ...and viewer absent entirely. + buildFeedPost(uri: postUriB), + ], + ), + ]); + + final feed = newFeedProvider( + votes: voteProvider, + subscriptions: subscriptions, + ); + await feed.loadFeed(FeedType.discover, refresh: true); + + // Not coerced to false: the snapshot said nothing, so nothing is said. + expect(subscriptions.isSubscribed(communityDid), true); + }); + + test('C4 hydrates nothing when unauthenticated, and everything from the ' + 'same response when authenticated', () async { + stubDiscoverPages([ + TimelineResponse( + feed: [ + buildFeedPost( + uri: postUriA, + vote: 'up', + voteUri: voteUri, + communityViewer: CommunityRefViewerState(subscribed: true), + ), + ], + ), + ]); + + final signedOutSubscriptions = newSubscriptionProvider(); + when(mockAuthProvider.isAuthenticated).thenReturn(false); + final signedOutFeed = newFeedProvider( + votes: voteProvider, + subscriptions: signedOutSubscriptions, + ); + await signedOutFeed.loadFeed(FeedType.discover, refresh: true); + + expect(voteProvider.isLiked(postUriA), false); + expect(voteProvider.getVoteState(postUriA), isNull); + expect(signedOutSubscriptions.isSubscribed(communityDid), false); + + // Positive control: identical input, authenticated. + final signedInSubscriptions = newSubscriptionProvider(); + when(mockAuthProvider.isAuthenticated).thenReturn(true); + final signedInFeed = newFeedProvider( + votes: voteProvider, + subscriptions: signedInSubscriptions, + ); + await signedInFeed.loadFeed(FeedType.discover, refresh: true); + + expect(voteProvider.isLiked(postUriA), true); + expect(voteProvider.getVoteState(postUriA)?.uri, voteUri); + expect(signedInSubscriptions.isSubscribed(communityDid), true); + }); + + test('C5 skips vote hydration when the vote provider is absent while ' + 'still applying subscriptions', () async { + final subscriptions = newSubscriptionProvider(); + stubDiscoverPages([ + TimelineResponse( + feed: [ + buildFeedPost( + uri: postUriA, + vote: 'up', + voteUri: voteUri, + communityViewer: CommunityRefViewerState(subscribed: true), + ), + ], + ), + ]); + + final feed = newFeedProvider(subscriptions: subscriptions); + await feed.loadFeed(FeedType.discover, refresh: true); + + // The unwired provider never hears about the vote... + expect(voteProvider.isLiked(postUriA), false); + // ...but the loop did run: subscriptions landed. + expect(subscriptions.isSubscribed(communityDid), true); + }); + + test('C12a hydrates a cursor-drift duplicate because it iterates the ' + 'raw response feed', () async { + stubDiscoverPages([ + TimelineResponse( + feed: [buildFeedPost(uri: postUriA, vote: 'up', voteUri: voteUri)], + cursor: 'page-2', + ), + // Cursor drift re-delivers the same post with the vote gone. No + // local mutation is outstanding, so the snapshot is adopted. + TimelineResponse( + feed: [buildFeedPost(uri: postUriA), buildFeedPost(uri: postUriB)], + ), + ]); + + final feed = newFeedProvider(votes: voteProvider); + await feed.loadFeed(FeedType.discover, refresh: true); + expect(voteProvider.isLiked(postUriA), true); + + await feed.loadMore(FeedType.discover); + + // DIVERGENCE (D3): the duplicate IS hydrated here. The + // CursorPaginationController-backed sites skip it - see C12b. + expect(voteProvider.isLiked(postUriA), false); + }); + }); + + group('profile posts hydration - votes only (UserProfileProvider)', () { + test('C6 applies votes but never subscriptions, even though the feed ' + 'carries community viewer state and the subscription surface is ' + 'fully wired', () async { + final subscriptions = newSubscriptionProvider(); + // Deliberately wired for BOTH votes and subscriptions, and handed to + // the profile provider directly: "no subscription seeded" below can + // then only come from the traversal this surface chose, never from a + // dependency that happened to be missing. + final hydrator = ViewerStateHydrator( + authProvider: mockAuthProvider, + voteProvider: voteProvider, + subscriptionProvider: subscriptions, + ); + + final posts = [ + buildFeedPost( + uri: postUriA, + vote: 'up', + voteUri: voteUri, + communityViewer: CommunityRefViewerState(subscribed: true), + ), + ]; + stubAuthorPostsPages([TimelineResponse(feed: posts)]); + + final profile = await newProfileProvider(hydrator: hydrator); + await profile.loadPosts(refresh: true); + + expect(voteProvider.isLiked(postUriA), true); + // DIVERGENCE (D1): profile posts hydrate votes only. If this ever + // starts passing subscriptions through, that is a behaviour change. + expect(subscriptions.isSubscribed(communityDid), false); + + // Positive control: the SAME hydrator over the SAME posts through the + // votes-and-subscriptions traversal does seed. So the assertion above + // is about which traversal the profile surface picks - not about an + // unwired provider or an input that says nothing. + hydrator.hydrateFeed(posts); + expect(subscriptions.isSubscribed(communityDid), true); + }); + + test('C5 profile posts load without hydrating when no vote provider is ' + 'wired', () async { + stubAuthorPostsPages([ + TimelineResponse( + feed: [buildFeedPost(uri: postUriA, vote: 'up', voteUri: voteUri)], + ), + ]); + + // Positive control FIRST: the identical page through a wired hydrator + // does reach the vote surface, so "nothing was hydrated" below cannot + // be blamed on an input that carries nothing. + final wiredVotes = MockVoteProvider(); + final wired = await newProfileProvider( + hydrator: ViewerStateHydrator( + authProvider: mockAuthProvider, + voteProvider: wiredVotes, + ), + ); + await wired.loadPosts(refresh: true); + verify( + wiredVotes.applyServerVoteState( + postUri: postUriA, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + + // With no vote provider there is nothing to verify against - the + // whole point is that no collaborator exists - so the assertion is + // that the page still loads rather than dereferencing a null. + // (viewer_state_hydrator_test.dart covers the null-provider gate + // directly, on the hydrator itself.) + final unwired = await newProfileProvider( + hydrator: ViewerStateHydrator(authProvider: mockAuthProvider), + ); + await unwired.loadPosts(refresh: true); + + expect(unwired.postsState.error, isNull); + expect(unwired.postsState.posts, hasLength(1)); + }); + + test('C12b skips a cursor-drift duplicate because the pagination ' + 'controller only hands over deduplicated new items', () async { + stubAuthorPostsPages([ + TimelineResponse( + feed: [buildFeedPost(uri: postUriA, vote: 'up', voteUri: voteUri)], + cursor: 'page-2', + ), + TimelineResponse( + feed: [buildFeedPost(uri: postUriA), buildFeedPost(uri: postUriB)], + ), + ]); + + final profile = await newProfileProvider(votes: voteProvider); + await profile.loadPosts(refresh: true); + expect(voteProvider.isLiked(postUriA), true); + + await profile.loadMorePosts(); + + // DIVERGENCE (D3), the other half of C12a: the duplicate was dropped + // before hydration, so its stale snapshot never lands. + expect(voteProvider.isLiked(postUriA), true); + expect(voteProvider.getVoteState(postUriA)?.uri, voteUri); + // The genuinely new item on the same page still hydrates. + expect(voteProvider.isLiked(postUriB), false); + expect(profile.postsState.posts, hasLength(2)); + }); + }); + + group('profile comments hydration - flat list (UserProfileProvider)', () { + const commentUriA = 'at://did:plc:author/social.coves.comment.record/c1'; + const commentUriB = 'at://did:plc:author/social.coves.comment.record/c2'; + + test('C7 applies the vote snapshot for every comment, including a null ' + 'direction', () async { + voteProvider.applyServerVoteState( + postUri: commentUriB, + voteDirection: 'up', + voteUri: voteUri, + ); + + stubActorComments([ + buildComment(uri: commentUriA, vote: 'up', voteUri: voteUri), + buildComment(uri: commentUriB), + ]); + + final profile = await newProfileProvider(votes: voteProvider); + await profile.loadComments(refresh: true); + + expect(voteProvider.isLiked(commentUriA), true); + expect(voteProvider.getVoteState(commentUriA)?.uri, voteUri); + expect(voteProvider.isLiked(commentUriB), false); + }); + + test('C5 profile comments load without hydrating when no vote provider ' + 'is wired', () async { + stubActorComments([ + buildComment(uri: commentUriA, vote: 'up', voteUri: voteUri), + ]); + + // Positive control: the same page through a wired hydrator does hand + // the snapshot over. + final wiredVotes = MockVoteProvider(); + final wired = await newProfileProvider( + hydrator: ViewerStateHydrator( + authProvider: mockAuthProvider, + voteProvider: wiredVotes, + ), + ); + await wired.loadComments(refresh: true); + verify( + wiredVotes.applyServerVoteState( + postUri: commentUriA, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + + final unwired = await newProfileProvider( + hydrator: ViewerStateHydrator(authProvider: mockAuthProvider), + ); + await unwired.loadComments(refresh: true); + + expect(unwired.commentsState.error, isNull); + expect(unwired.commentsState.comments, hasLength(1)); + }); + }); + + group('comment tree hydration - recursive (CommentsProvider)', () { + const rootUri = 'at://did:plc:author/social.coves.community.comment/root'; + const childUri = 'at://did:plc:author/social.coves.community.comment/kid'; + const grandchildUri = + 'at://did:plc:author/social.coves.community.comment/gk'; + + test('C8 recurses to every depth of the delivered tree', () async { + stubThreadResponse([ + buildThreadComment( + uri: rootUri, + vote: 'up', + voteUri: voteUri, + replies: [ + buildThreadComment( + uri: childUri, + vote: 'up', + voteUri: voteUri, + replies: [ + buildThreadComment( + uri: grandchildUri, + vote: 'up', + voteUri: voteUri, + ), + ], + ), + ], + ), + ]); + + final comments = newCommentsProvider(votes: voteProvider); + await comments.loadComments(refresh: true); + + expect(voteProvider.isLiked(rootUri), true); + expect(voteProvider.isLiked(childUri), true); + expect(voteProvider.isLiked(grandchildUri), true); + expect(voteProvider.getVoteState(grandchildUri)?.uri, voteUri); + }); + + test('C5 the comment tree loads without hydrating when no vote provider ' + 'is wired', () async { + stubThreadResponse([ + buildThreadComment(uri: rootUri, vote: 'up', voteUri: voteUri), + ]); + + // Positive control: the same tree through a wired hydrator does hand + // the snapshot over. + final wiredVotes = MockVoteProvider(); + final wired = newCommentsProvider( + hydrator: ViewerStateHydrator( + authProvider: mockAuthProvider, + voteProvider: wiredVotes, + ), + ); + await wired.loadComments(refresh: true); + verify( + wiredVotes.applyServerVoteState( + postUri: rootUri, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + + final comments = newCommentsProvider( + hydrator: ViewerStateHydrator(authProvider: mockAuthProvider), + ); + await comments.loadComments(refresh: true); + + expect(comments.error, isNull); + expect(comments.comments, hasLength(1)); + }); + + test('C9 applies only the nodes the response delivered, so a preserved ' + 'stale branch cannot roll back a confirmed vote', () async { + stubThreadResponse([ + buildThreadComment( + uri: rootUri, + replies: [buildThreadComment(uri: childUri)], + ), + ]); + + final comments = newCommentsProvider(votes: voteProvider); + await comments.loadComments(refresh: true); + + // The user likes the child; the appview has not indexed it yet. + await voteProvider.toggleVote(postUri: childUri, postCid: 'cid-child'); + expect(voteProvider.getAdjustedScore(childUri, 5), 6); + + // Another surface sharing the VoteProvider confirms the vote, which + // clears the outstanding adjustment. + voteProvider.applyServerVoteState( + postUri: childUri, + voteDirection: 'up', + voteUri: voteUri, + ); + expect(voteProvider.getAdjustedScore(childUri, 6), 6); + + // Load-more on the root hits the depth cutoff: the response carries no + // replies, so the merge preserves the stale child branch. + stubSubtreeResponse(buildThreadComment(uri: rootUri)); + final subtree = await comments.loadMoreReplies(rootUri); + + expect( + subtree!.replies!.map((r) => r.comment.uri), + contains(childUri), + ); + // The preserved node was never delivered, so it was never applied. + expect(voteProvider.isLiked(childUri), true); + expect(voteProvider.getAdjustedScore(childUri, 6), 6); + }); + }); +} diff --git a/test/screens/communities_admin_panel_characterization_test.dart b/test/screens/communities_admin_panel_characterization_test.dart new file mode 100644 index 0000000..1c8b668 --- /dev/null +++ b/test/screens/communities_admin_panel_characterization_test.dart @@ -0,0 +1,503 @@ +// Characterization net for CommunitiesAdminPanel, ahead of its split into a +// thin shell + admin menu + create form + avatar upload page + a shared text +// field + a pure name validator. +// +// Everything is asserted through the rendered UI, because every piece of the +// state the split will move - _currentPage, _nameError, _selectedCommunity, +// _selectedImage, _isLoadingCommunities - is private today. +// +// Two behaviours here look like bugs and are not: +// +// * The name validator LOWERCASES before matching its regex, so an +// uppercase name is ACCEPTED and sent lowercased, despite the error copy +// promising "must be lowercase letters" (A3/A10). +// * All three create-form fields share ONE listener, so typing in the +// Description field clears an error raised against the Name field (A12). +// Giving each extracted field its own listener destroys that. + +import 'dart:async'; + +import 'package:coves_flutter/models/community.dart'; +import 'package:coves_flutter/screens/home/communities_admin_panel.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; + +import '../test_helpers/test_mocks.dart'; + +const String communityDid = 'did:plc:testcove'; + +CommunityView buildCommunity({ + String did = communityDid, + String name = 'testcove', + String displayName = 'Test Cove', +}) { + return CommunityView( + did: did, + name: name, + displayName: displayName, + handle: '$name.coves.social', + ); +} + +CreateCommunityResponse buildCreateResponse() { + return const CreateCommunityResponse( + uri: 'at://did:plc:new/social.coves.community/1', + cid: 'bafycommunity', + did: 'did:plc:new', + handle: 'c-worldnews.coves.social', + ); +} + +void main() { + late MockCovesApiService mockApiService; + + setUp(() { + mockApiService = MockCovesApiService(); + }); + + Future pumpPanel(WidgetTester tester) async { + await tester.pumpWidget( + Provider.value( + value: mockApiService, + child: const MaterialApp(home: CommunitiesAdminPanel()), + ), + ); + await tester.pumpAndSettle(); + } + + void stubListCommunities(List communities) { + when( + mockApiService.listCommunities( + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + sort: anyNamed('sort'), + subscribed: anyNamed('subscribed'), + ), + ).thenAnswer((_) async => CommunitiesResponse(communities: communities)); + } + + void stubCreateCommunity() { + when( + mockApiService.createCommunity( + name: anyNamed('name'), + displayName: anyNamed('displayName'), + description: anyNamed('description'), + visibility: anyNamed('visibility'), + ), + ).thenAnswer((_) async => buildCreateResponse()); + } + + // The create form's three fields, in render order: name, display name, + // description. Asserted to be exactly three wherever it matters. + Finder nameField() => find.byType(TextField).at(0); + Finder displayNameField() => find.byType(TextField).at(1); + Finder descriptionField() => find.byType(TextField).at(2); + + Finder createButton() => + find.widgetWithText(ElevatedButton, 'Create Community'); + + Finder backArrow() => find.byTooltip('Back'); + + bool createEnabled(WidgetTester tester) => + tester.widget(createButton()).onPressed != null; + + /// Opens the create form from the menu. + Future openCreateForm(WidgetTester tester) async { + await pumpPanel(tester); + await tester.tap(find.text('Create Community')); + await tester.pumpAndSettle(); + expect(find.byType(TextField), findsNWidgets(3)); + } + + /// Taps submit, scrolling it into view first: the form is a + /// SingleChildScrollView and the button sits below the fold at the test + /// viewport size, where a tap would silently miss. + Future tapCreate(WidgetTester tester) async { + await tester.ensureVisible(createButton()); + await tester.pumpAndSettle(); + await tester.tap(createButton()); + await tester.pumpAndSettle(); + } + + /// Fills all three fields, which is what enables the submit button. + Future fillForm( + WidgetTester tester, { + required String name, + String displayName = 'World News', + String description = 'Global news', + }) async { + await tester.enterText(nameField(), name); + await tester.enterText(displayNameField(), displayName); + await tester.enterText(descriptionField(), description); + await tester.pumpAndSettle(); + } + + group('name validation, driven through the submit button', () { + testWidgets('A1 the "Name is required" branch is unreachable: a blank ' + 'name disables submit before the validator can run', (tester) async { + // The submit guard requires all three fields non-empty AFTER + // trimming, and toLowerCase() cannot empty a non-empty string, so the + // validator's empty-name branch is dead through this widget. What is + // observable is the guard that makes it dead. Once the validator is + // extracted as a pure function the branch becomes directly testable; + // it is not today. + await openCreateForm(tester); + await tester.enterText(displayNameField(), 'World News'); + await tester.enterText(descriptionField(), 'Global news'); + await tester.enterText(nameField(), ' '); + await tester.pumpAndSettle(); + + expect(createEnabled(tester), isFalse); + expect(find.text('Name is required'), findsNothing); + }); + + testWidgets('A2 a name longer than 63 characters is rejected', ( + tester, + ) async { + await openCreateForm(tester); + await fillForm(tester, name: 'a' * 64); + + await tapCreate(tester); + + expect( + find.text('Name must be 63 characters or less'), + findsOneWidget, + ); + verifyNever( + mockApiService.createCommunity( + name: anyNamed('name'), + displayName: anyNamed('displayName'), + description: anyNamed('description'), + visibility: anyNamed('visibility'), + ), + ); + }); + + testWidgets('A3 an uppercase name is ACCEPTED and sent lowercased', ( + tester, + ) async { + // The validator lowercases before matching, so the error copy's + // "must be lowercase letters" is a promise the code does not keep. + stubCreateCommunity(); + await openCreateForm(tester); + await fillForm(tester, name: 'MyCommunity'); + + await tapCreate(tester); + + expect( + find.text('Name must be lowercase letters, numbers, and hyphens only'), + findsNothing, + ); + verify( + mockApiService.createCommunity( + name: 'mycommunity', + displayName: 'World News', + description: 'Global news', + visibility: anyNamed('visibility'), + ), + ).called(1); + + // Let the success SnackBar time out so no timer outlives the test. + await tester.pumpAndSettle(const Duration(seconds: 5)); + }); + + testWidgets('A4 hyphens at the edges, underscores, spaces and dots are ' + 'rejected', (tester) async { + const rejected = [ + '-worldnews', + 'worldnews-', + 'world_news', + 'world news', + 'world.news', + ]; + + await openCreateForm(tester); + + for (final name in rejected) { + // Retyping clears the previous error (the shared listener), so each + // iteration genuinely re-raises it rather than reading a stale one. + await fillForm(tester, name: name); + expect( + find.text( + 'Name must be lowercase letters, numbers, and hyphens only', + ), + findsNothing, + reason: 'typing should have cleared the previous error', + ); + + await tapCreate(tester); + + expect( + find.text( + 'Name must be lowercase letters, numbers, and hyphens only', + ), + findsOneWidget, + reason: '"$name" should be rejected by the charset rule', + ); + } + + verifyNever( + mockApiService.createCommunity( + name: anyNamed('name'), + displayName: anyNamed('displayName'), + description: anyNamed('description'), + visibility: anyNamed('visibility'), + ), + ); + }); + + testWidgets('A5 a valid name passes and reaches the API', (tester) async { + stubCreateCommunity(); + await openCreateForm(tester); + // Interior hyphens and digits are fine. + await fillForm(tester, name: 'world-news-2'); + + await tapCreate(tester); + + expect(find.textContaining('Name must be'), findsNothing); + verify( + mockApiService.createCommunity( + name: 'world-news-2', + displayName: anyNamed('displayName'), + description: anyNamed('description'), + visibility: anyNamed('visibility'), + ), + ).called(1); + + await tester.pumpAndSettle(const Duration(seconds: 5)); + }); + }); + + group('page routing', () { + testWidgets('A6 the menu renders by default, with no back arrow', ( + tester, + ) async { + await pumpPanel(tester); + + expect(find.text('Admin Tools'), findsOneWidget); + expect(find.text('Create Community'), findsOneWidget); + expect(find.text('Change Profile Pic'), findsOneWidget); + expect(find.byType(TextField), findsNothing); + // The menu is the root page: leaving is the shell's business. + expect(backArrow(), findsNothing); + }); + + testWidgets('A7 tapping Create Community shows the form, and the in-app ' + 'back arrow returns to the menu', (tester) async { + await pumpPanel(tester); + + await tester.tap(find.text('Create Community')); + await tester.pumpAndSettle(); + + expect(find.byType(TextField), findsNWidgets(3)); + expect(find.text('Admin Tools'), findsNothing); + expect(backArrow(), findsOneWidget); + + await tester.tap(backArrow()); + await tester.pumpAndSettle(); + + expect(find.text('Admin Tools'), findsOneWidget); + expect(find.byType(TextField), findsNothing); + expect(backArrow(), findsNothing); + }); + + testWidgets('A8 navigating to Change Profile Pic loads communities, and ' + 're-entering while that load is in flight does not refetch', ( + tester, + ) async { + final inFlight = Completer(); + when( + mockApiService.listCommunities( + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + sort: anyNamed('sort'), + subscribed: anyNamed('subscribed'), + ), + ).thenAnswer((_) => inFlight.future); + + await pumpPanel(tester); + await tester.tap(find.text('Change Profile Pic')); + // pump, not pumpAndSettle: the loading spinner never stops animating. + await tester.pump(); + + expect(find.text('Change Profile Picture'), findsOneWidget); + expect(backArrow(), findsOneWidget); + verify( + mockApiService.listCommunities( + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + sort: anyNamed('sort'), + subscribed: anyNamed('subscribed'), + ), + ).called(1); + + // Bounce back to the menu and re-enter while the first load is still + // outstanding: the in-flight guard must swallow the second attempt. + await tester.tap(backArrow()); + await tester.pump(); + await tester.tap(find.text('Change Profile Pic')); + await tester.pump(); + + verifyNever( + mockApiService.listCommunities( + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + sort: anyNamed('sort'), + subscribed: anyNamed('subscribed'), + ), + ); + + inFlight.complete(CommunitiesResponse(communities: [buildCommunity()])); + await tester.pumpAndSettle(); + expect(find.text('Test Cove'), findsOneWidget); + }); + }); + + group('create form state', () { + testWidgets('A9 submit stays disabled until all three fields are ' + 'non-empty', (tester) async { + await openCreateForm(tester); + expect(createEnabled(tester), isFalse); + + await tester.enterText(nameField(), 'worldnews'); + await tester.pumpAndSettle(); + expect(createEnabled(tester), isFalse); + + await tester.enterText(displayNameField(), 'World News'); + await tester.pumpAndSettle(); + expect(createEnabled(tester), isFalse); + + await tester.enterText(descriptionField(), 'Global news'); + await tester.pumpAndSettle(); + expect(createEnabled(tester), isTrue); + + // Whitespace does not count as filled: the form trims first. + await tester.enterText(descriptionField(), ' '); + await tester.pumpAndSettle(); + expect(createEnabled(tester), isFalse); + }); + + testWidgets('A10 the handle preview tracks the name field and lowercases ' + 'it', (tester) async { + await openCreateForm(tester); + expect(find.text('@c-{name}.coves.social'), findsOneWidget); + + await tester.enterText(nameField(), 'worldnews'); + await tester.pumpAndSettle(); + expect(find.text('@c-worldnews.coves.social'), findsOneWidget); + + // Same lowercasing the validator and the create call apply. + await tester.enterText(nameField(), 'MyCove'); + await tester.pumpAndSettle(); + expect(find.text('@c-mycove.coves.social'), findsOneWidget); + + await tester.enterText(nameField(), ''); + await tester.pumpAndSettle(); + expect(find.text('@c-{name}.coves.social'), findsOneWidget); + }); + + testWidgets('A12 the three fields share one listener: typing in ' + 'Description clears an error raised against Name', (tester) async { + await openCreateForm(tester); + await fillForm(tester, name: 'bad name'); + + await tapCreate(tester); + expect( + find.text('Name must be lowercase letters, numbers, and hyphens only'), + findsOneWidget, + ); + + // Type into a DIFFERENT field. One shared listener means the name + // error clears; per-field listeners would leave it on screen. + await tester.enterText(descriptionField(), 'Global news updated'); + await tester.pumpAndSettle(); + + expect( + find.text('Name must be lowercase letters, numbers, and hyphens only'), + findsNothing, + ); + // The name itself is untouched - only the error was cleared. + expect( + tester.widget(nameField()).controller?.text, + 'bad name', + ); + }); + }); + + group('avatar upload page state', () { + testWidgets('A11 going back to the menu clears the selected community', ( + tester, + ) async { + stubListCommunities([buildCommunity()]); + await pumpPanel(tester); + + await tester.tap(find.text('Change Profile Pic')); + await tester.pumpAndSettle(); + expect(find.text('Test Cove'), findsOneWidget); + expect(find.text('Current Profile Picture'), findsNothing); + + // Selecting a community reveals its current picture section. No image + // picker is involved, so the platform statics are never reached. + await tester.tap(find.text('Test Cove')); + await tester.pumpAndSettle(); + expect(find.text('Current Profile Picture'), findsOneWidget); + + await tester.tap(backArrow()); + await tester.pumpAndSettle(); + expect(find.text('Admin Tools'), findsOneWidget); + + // Re-entering starts from a clean slate. + await tester.tap(find.text('Change Profile Pic')); + await tester.pumpAndSettle(); + expect(find.text('Test Cove'), findsOneWidget); + expect(find.text('Current Profile Picture'), findsNothing); + }); + + testWidgets('an empty community list renders the empty state', ( + tester, + ) async { + stubListCommunities(const []); + await pumpPanel(tester); + + await tester.tap(find.text('Change Profile Pic')); + await tester.pumpAndSettle(); + + expect(find.text('No communities found'), findsOneWidget); + expect(find.text('Current Profile Picture'), findsNothing); + }); + }); + + group('teardown', () { + testWidgets('unmounting from any page tears down cleanly', (tester) async { + // A controller whose dispose() is dropped by the split, or a listener + // left attached to a disposed State, surfaces here as a thrown + // FlutterError during finalization. Per-controller disposal is not + // otherwise observable: all four controllers are private, and the + // fourth (_communityHandleController) is never attached to any field. + stubListCommunities([buildCommunity()]); + + await pumpPanel(tester); + await tester.pumpWidget(const MaterialApp(home: Scaffold())); + expect(tester.takeException(), isNull); + + await pumpPanel(tester); + await tester.tap(find.text('Create Community')); + await tester.pumpAndSettle(); + await tester.enterText(nameField(), 'worldnews'); + await tester.pumpAndSettle(); + await tester.pumpWidget(const MaterialApp(home: Scaffold())); + expect(tester.takeException(), isNull); + + await pumpPanel(tester); + await tester.tap(find.text('Change Profile Pic')); + await tester.pumpAndSettle(); + await tester.pumpWidget(const MaterialApp(home: Scaffold())); + expect(tester.takeException(), isNull); + + await tester.pumpAndSettle(); + }); + }); +} diff --git a/test/screens/communities_admin_panel_draft_persistence_test.dart b/test/screens/communities_admin_panel_draft_persistence_test.dart new file mode 100644 index 0000000..b54be8e --- /dev/null +++ b/test/screens/communities_admin_panel_draft_persistence_test.dart @@ -0,0 +1,135 @@ +// The admin panel's create-form draft and receipt list must survive a trip +// to the menu. +// +// CreateCommunityForm is built only while the shell's current page is +// AdminPage.createCommunity, so its State is disposed on every back-tap. +// Anything the form owned outright would be thrown away the moment the admin +// glanced at the menu. Two things must not be: +// +// * the in-progress draft (the three text controllers), and +// * the green "Created Communities" receipts, which are the admin's only +// record that a community was created. +// +// Both therefore live on the panel shell, which outlives the page, and are +// passed down. These tests pin that ownership split from the outside: they +// never name the shell's fields, only the behaviour a user would notice. +// +// This is the boundary a future refactor is most likely to get wrong again - +// moving either back into the form reads like tidying up and silently +// destroys work the admin has done. + +import 'package:coves_flutter/models/community.dart'; +import 'package:coves_flutter/screens/home/communities_admin_panel.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; + +import '../test_helpers/test_mocks.dart'; + +const CreateCommunityResponse createdCommunity = CreateCommunityResponse( + uri: 'at://did:plc:new/social.coves.community/1', + cid: 'bafycommunity', + did: 'did:plc:newcommunity', + handle: 'c-worldnews.coves.social', +); + +void main() { + late MockCovesApiService mockApiService; + + setUp(() { + mockApiService = MockCovesApiService(); + when( + mockApiService.createCommunity( + name: anyNamed('name'), + displayName: anyNamed('displayName'), + description: anyNamed('description'), + visibility: anyNamed('visibility'), + ), + ).thenAnswer((_) async => createdCommunity); + }); + + Finder nameField() => find.byType(TextField).at(0); + Finder displayNameField() => find.byType(TextField).at(1); + Finder descriptionField() => find.byType(TextField).at(2); + Finder backArrow() => find.byTooltip('Back'); + Finder createButton() => + find.widgetWithText(ElevatedButton, 'Create Community'); + + String textOf(WidgetTester tester, Finder field) => + tester.widget(field).controller?.text ?? ''; + + Future pumpPanel(WidgetTester tester) async { + await tester.pumpWidget( + Provider.value( + value: mockApiService, + child: const MaterialApp(home: CommunitiesAdminPanel()), + ), + ); + await tester.pumpAndSettle(); + } + + /// Menu -> create form. + Future openCreateForm(WidgetTester tester) async { + await tester.tap(find.text('Create Community')); + await tester.pumpAndSettle(); + expect(find.byType(TextField), findsNWidgets(3)); + } + + /// Create form -> menu, via the in-app back arrow. + Future tapBack(WidgetTester tester) async { + await tester.tap(backArrow()); + await tester.pumpAndSettle(); + expect(find.text('Admin Tools'), findsOneWidget); + } + + testWidgets('a half-typed draft survives a trip to the menu and back', ( + tester, + ) async { + await pumpPanel(tester); + await openCreateForm(tester); + + await tester.enterText(nameField(), 'worldnews'); + await tester.enterText(displayNameField(), 'World News'); + await tester.enterText(descriptionField(), 'Global news'); + await tester.pumpAndSettle(); + + await tapBack(tester); + await openCreateForm(tester); + + // The user stepped out to the menu for a moment; their draft must not + // have been thrown away. + expect(textOf(tester, nameField()), 'worldnews'); + expect(textOf(tester, displayNameField()), 'World News'); + expect(textOf(tester, descriptionField()), 'Global news'); + }); + + testWidgets('the created-communities receipt survives a trip to the menu ' + 'and back', (tester) async { + await pumpPanel(tester); + await openCreateForm(tester); + + await tester.enterText(nameField(), 'worldnews'); + await tester.enterText(displayNameField(), 'World News'); + await tester.enterText(descriptionField(), 'Global news'); + await tester.pumpAndSettle(); + + await tester.ensureVisible(createButton()); + await tester.pumpAndSettle(); + await tester.tap(createButton()); + // Let the success SnackBar come and go. + await tester.pumpAndSettle(const Duration(seconds: 5)); + + expect(find.text('Created Communities'), findsOneWidget); + expect(find.text(createdCommunity.handle), findsOneWidget); + + await tapBack(tester); + await openCreateForm(tester); + + // The receipt is the only record the admin has that the community was + // created; it must not vanish because they looked at the menu. + expect(find.text('Created Communities'), findsOneWidget); + expect(find.text(createdCommunity.handle), findsOneWidget); + }); +} diff --git a/test/screens/communities_admin_panel_inflight_test.dart b/test/screens/communities_admin_panel_inflight_test.dart new file mode 100644 index 0000000..c3e3b21 --- /dev/null +++ b/test/screens/communities_admin_panel_inflight_test.dart @@ -0,0 +1,175 @@ +// RED: the contract for a second regression the admin-panel split +// introduced, this time for an in-flight create. +// +// These tests FAIL against the code as it stands. Do not soften them. +// +// Pre-split, _createCommunity ran on the panel's own State, which survives a +// page switch, so `mounted` stayed true for the whole request and the +// success block always ran. +// +// Post-split it runs on the disposable _CreateCommunityFormState. Tap back +// to the menu while createCommunity is in flight and that State is disposed, +// so `mounted` is false and create_community_form.dart:186 skips the ENTIRE +// success block: onCommunityCreated never fires (no receipt), the fields are +// never cleared, no confirmation is shown. +// +// The server call already succeeded. Because the fields still hold the +// draft, returning to the form builds a fresh State with _isSubmitting +// false, an enabled submit button, and no memory that anything was created - +// so the admin can create a DUPLICATE community. +// +// The fix has to make the outcome of a succeeded request reach the shell +// regardless of whether the page that started it is still mounted. + +import 'dart:async'; + +import 'package:coves_flutter/models/community.dart'; +import 'package:coves_flutter/screens/home/communities_admin_panel.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; + +import '../test_helpers/test_mocks.dart'; + +const CreateCommunityResponse createdCommunity = CreateCommunityResponse( + uri: 'at://did:plc:new/social.coves.community/1', + cid: 'bafycommunity', + did: 'did:plc:newcommunity', + handle: 'c-worldnews.coves.social', +); + +void main() { + late MockCovesApiService mockApiService; + late Completer gate; + + setUp(() { + mockApiService = MockCovesApiService(); + // NOTE: `gate` is deliberately NOT created here. setUp runs outside the + // FakeAsync zone testWidgets wraps the test body in, so a Completer + // built here hands back a future bound to the OUTER zone - and + // tester.pump() never flushes that queue, so the continuation lands + // after the assertions have already run. Each test creates its own gate + // as its first statement, inside the zone. The stub below only captures + // the variable, so it is safe to register early. + when( + mockApiService.createCommunity( + name: anyNamed('name'), + displayName: anyNamed('displayName'), + description: anyNamed('description'), + visibility: anyNamed('visibility'), + ), + ).thenAnswer((_) => gate.future); + }); + + Finder nameField() => find.byType(TextField).at(0); + Finder displayNameField() => find.byType(TextField).at(1); + Finder descriptionField() => find.byType(TextField).at(2); + Finder backArrow() => find.byTooltip('Back'); + Finder createButton() => + find.widgetWithText(ElevatedButton, 'Create Community'); + + String textOf(WidgetTester tester, Finder field) => + tester.widget(field).controller?.text ?? ''; + + VerificationResult verifyCreateCalls() => verify( + mockApiService.createCommunity( + name: anyNamed('name'), + displayName: anyNamed('displayName'), + description: anyNamed('description'), + visibility: anyNamed('visibility'), + ), + ); + + Future pumpPanel(WidgetTester tester) async { + await tester.pumpWidget( + Provider.value( + value: mockApiService, + child: const MaterialApp(home: CommunitiesAdminPanel()), + ), + ); + await tester.pumpAndSettle(); + } + + /// Menu -> create form. + Future openCreateForm(WidgetTester tester) async { + await tester.tap(find.text('Create Community')); + await tester.pumpAndSettle(); + expect(find.byType(TextField), findsNWidgets(3)); + } + + /// Fills the form and submits, leaving the request in flight. + /// + /// Only pumps a single frame: once submitting, the button hosts a + /// CircularProgressIndicator whose animation never settles. + Future submitAndLeaveInFlight(WidgetTester tester) async { + await tester.enterText(nameField(), 'worldnews'); + await tester.enterText(displayNameField(), 'World News'); + await tester.enterText(descriptionField(), 'Global news'); + await tester.pumpAndSettle(); + + // The submit button sits below the fold at the test viewport size. + await tester.ensureVisible(createButton()); + await tester.pumpAndSettle(); + await tester.tap(createButton()); + await tester.pump(); + } + + /// Back to the menu, then resolve the request that is still outstanding. + Future leaveThenCompleteRequest(WidgetTester tester) async { + await tester.tap(backArrow()); + await tester.pump(); + expect(find.text('Admin Tools'), findsOneWidget); + + gate.complete(createdCommunity); + // Let the awaiting continuation run against the now-disposed State. + await tester.pump(); + await tester.pump(); + } + + testWidgets('a create that succeeds after the user returned to the menu ' + 'still records its receipt and clears the draft', (tester) async { + gate = Completer(); + + await pumpPanel(tester); + await openCreateForm(tester); + await submitAndLeaveInFlight(tester); + await leaveThenCompleteRequest(tester); + + await openCreateForm(tester); + + // The community exists on the server; the admin's only record of that + // is the receipt, so it must be there. + expect(find.text('Created Communities'), findsOneWidget); + expect(find.text(createdCommunity.handle), findsOneWidget); + + // And the draft that produced it must not still be sitting in the form + // inviting a re-submit. + expect(textOf(tester, nameField()), ''); + expect(textOf(tester, displayNameField()), ''); + expect(textOf(tester, descriptionField()), ''); + }); + + testWidgets('retrying after a create that succeeded off-page does not ' + 'create a duplicate community', (tester) async { + gate = Completer(); + + await pumpPanel(tester); + await openCreateForm(tester); + await submitAndLeaveInFlight(tester); + await leaveThenCompleteRequest(tester); + + await openCreateForm(tester); + + // The admin saw no confirmation, so they try again. Whatever the form + // chooses to do about that - disable submit, clear the draft, dedupe - + // exactly one community may reach the server. + await tester.ensureVisible(createButton()); + await tester.pumpAndSettle(); + await tester.tap(createButton()); + await tester.pump(); + + verifyCreateCalls().called(1); + }); +} diff --git a/test/screens/viewer_state_hydration_screens_test.dart b/test/screens/viewer_state_hydration_screens_test.dart new file mode 100644 index 0000000..5636026 --- /dev/null +++ b/test/screens/viewer_state_hydration_screens_test.dart @@ -0,0 +1,393 @@ +// Characterization net for the viewer-state hydration performed inside +// widgets (sites 2, 7 and 8 of the eight hydration sites), driven through +// the real screens. +// +// Asserted only through CommunitySubscriptionProvider.isSubscribed and +// VoteProvider.isLiked / getVoteState, so the net survives the extraction +// of the hydration logic into a shared service. +// +// The two community-subscription sites disagree about a null `subscribed` +// and that disagreement is load bearing: +// +// * the LIST site skips a community whose `viewer` is null, but coerces a +// present-viewer's null `subscribed` to false; +// * the SINGLE-community site skips whenever `subscribed` is null. +// +// Both halves are pinned below. + +import 'package:coves_flutter/models/community.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/community_subscription_provider.dart'; +import 'package:coves_flutter/providers/vote_provider.dart'; +import 'package:coves_flutter/screens/community/community_feed_screen.dart'; +import 'package:coves_flutter/screens/home/communities_discovery_screen.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:coves_flutter/services/streamable_service.dart'; +import 'package:coves_flutter/services/vote_service.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; + +import '../test_helpers/test_mocks.dart'; + +class _FakeAuthProvider extends AuthProvider { + _FakeAuthProvider({required bool authenticated}) + : _isAuthenticated = authenticated; + + final bool _isAuthenticated; + + @override + bool get isAuthenticated => _isAuthenticated; + + @override + bool get isLoading => false; +} + +/// A subscription provider whose sign-in load is a no-op, so nothing races +/// the seeding under test. +class _TestSubscriptionProvider extends CommunitySubscriptionProvider { + _TestSubscriptionProvider({ + required super.authProvider, + required super.apiService, + }); + + @override + Future loadSubscribedCommunities() async {} +} + +const String communityDid = 'did:plc:community'; +const String communityHandle = 'testcove'; +const String postUri = 'at://did:plc:author/social.coves.community.post/p1'; +const String voteUri = 'at://did:plc:me/social.coves.feed.vote/v1'; + +CommunityView buildCommunity({CommunityViewerState? viewer}) { + return CommunityView( + did: communityDid, + name: communityHandle, + displayName: 'Test Cove', + viewer: viewer, + ); +} + +FeedViewPost buildFeedPost({ + String? vote, + String? refVoteUri, + CommunityRefViewerState? communityViewer, +}) { + return FeedViewPost( + post: PostView( + uri: postUri, + cid: 'cid-p1', + rkey: 'p1', + author: AuthorView(did: 'did:plc:author', handle: 'test.user'), + community: CommunityRef( + did: communityDid, + name: communityHandle, + viewer: communityViewer, + ), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + record: const PostRecord(title: 'Title', content: 'Body'), + stats: PostStats(upvotes: 0, downvotes: 0, score: 0, commentCount: 0), + viewer: ViewerState(vote: vote, voteUri: refVoteUri), + ), + ); +} + +void main() { + late MockCovesApiService mockApiService; + + setUp(() { + mockApiService = MockCovesApiService(); + }); + + VoteProvider buildVoteProvider(AuthProvider auth) { + final provider = VoteProvider( + voteService: VoteService( + sessionGetter: () async => null, + didGetter: () => null, + ), + authProvider: auth, + ); + addTearDown(provider.dispose); + return provider; + } + + CommunitySubscriptionProvider buildSubscriptionProvider(AuthProvider auth) { + final provider = _TestSubscriptionProvider( + authProvider: auth, + apiService: mockApiService, + ); + addTearDown(provider.dispose); + return provider; + } + + group('community LIST subscription seeding (discovery screen)', () { + void stubListCommunities(List communities) { + when( + mockApiService.listCommunities( + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + sort: anyNamed('sort'), + subscribed: anyNamed('subscribed'), + ), + ).thenAnswer((_) async => CommunitiesResponse(communities: communities)); + } + + Future pumpDiscovery( + WidgetTester tester, { + required bool seededSubscribed, + }) async { + final auth = _FakeAuthProvider(authenticated: true); + final subscriptions = buildSubscriptionProvider(auth) + ..setInitialSubscriptionState( + communityDid: communityDid, + isSubscribed: seededSubscribed, + ); + + await tester.pumpWidget( + MultiProvider( + providers: [ + Provider.value(value: mockApiService), + ChangeNotifierProvider.value(value: auth), + ChangeNotifierProvider.value( + value: subscriptions, + ), + ], + // The screen is normally embedded in MainShellScreen, which owns + // the Scaffold its Material widgets need. + child: const MaterialApp( + home: Scaffold(body: CommunitiesDiscoveryScreen()), + ), + ), + ); + await tester.pumpAndSettle(); + return subscriptions; + } + + testWidgets('C11a skips a community whose viewer is null, leaving known ' + 'state untouched', (tester) async { + stubListCommunities([buildCommunity()]); + + final subscriptions = await pumpDiscovery( + tester, + seededSubscribed: true, + ); + + expect(subscriptions.isSubscribed(communityDid), true); + }); + + testWidgets('C11b coerces a present viewer\'s null subscribed to false', ( + tester, + ) async { + stubListCommunities([buildCommunity(viewer: CommunityViewerState())]); + + final subscriptions = await pumpDiscovery( + tester, + seededSubscribed: true, + ); + + // DIVERGENCE: the single-community site (C11c) skips this same input. + expect(subscriptions.isSubscribed(communityDid), false); + }); + + testWidgets('a present viewer with subscribed true is applied', ( + tester, + ) async { + stubListCommunities([ + buildCommunity(viewer: CommunityViewerState(subscribed: true)), + ]); + + final subscriptions = await pumpDiscovery( + tester, + seededSubscribed: false, + ); + + expect(subscriptions.isSubscribed(communityDid), true); + }); + }); + + group('community feed screen hydration', () { + void stubCommunity(CommunityView community) { + when( + mockApiService.getCommunity(community: anyNamed('community')), + ).thenAnswer((_) async => community); + } + + void stubFeed(List feed) { + when( + mockApiService.getCommunityFeed( + community: anyNamed('community'), + sort: anyNamed('sort'), + timeframe: anyNamed('timeframe'), + limit: anyNamed('limit'), + cursor: anyNamed('cursor'), + ), + ).thenAnswer((_) async => TimelineResponse(feed: feed)); + } + + Future pumpFeedScreen( + WidgetTester tester, { + required AuthProvider auth, + required VoteProvider votes, + required CommunitySubscriptionProvider subscriptions, + }) async { + final blocks = BlockProvider( + apiService: mockApiService, + authProvider: auth, + ); + addTearDown(blocks.dispose); + + await tester.pumpWidget( + MultiProvider( + providers: [ + Provider.value(value: mockApiService), + ChangeNotifierProvider.value(value: auth), + ChangeNotifierProvider.value(value: votes), + ChangeNotifierProvider.value( + value: subscriptions, + ), + ChangeNotifierProvider.value(value: blocks), + Provider(create: (_) => StreamableService()), + ], + child: const MaterialApp( + home: CommunityFeedScreen(identifier: communityHandle), + ), + ), + ); + await tester.pumpAndSettle(); + } + + testWidgets('C11c skips the single community when subscribed is null, ' + 'leaving known state untouched', (tester) async { + stubCommunity(buildCommunity(viewer: CommunityViewerState())); + stubFeed(const []); + + final auth = _FakeAuthProvider(authenticated: true); + final votes = buildVoteProvider(auth); + final subscriptions = buildSubscriptionProvider(auth) + ..setInitialSubscriptionState( + communityDid: communityDid, + isSubscribed: true, + ); + + await pumpFeedScreen( + tester, + auth: auth, + votes: votes, + subscriptions: subscriptions, + ); + + // DIVERGENCE: the list site (C11b) coerces this same input to false. + expect(subscriptions.isSubscribed(communityDid), true); + }); + + testWidgets('the single community IS applied when subscribed is set', ( + tester, + ) async { + stubCommunity( + buildCommunity(viewer: CommunityViewerState(subscribed: false)), + ); + stubFeed(const []); + + final auth = _FakeAuthProvider(authenticated: true); + final votes = buildVoteProvider(auth); + final subscriptions = buildSubscriptionProvider(auth) + ..setInitialSubscriptionState( + communityDid: communityDid, + isSubscribed: true, + ); + + await pumpFeedScreen( + tester, + auth: auth, + votes: votes, + subscriptions: subscriptions, + ); + + expect(subscriptions.isSubscribed(communityDid), false); + }); + + testWidgets('C2/C1 the community feed page seeds both votes and ' + 'subscriptions when signed in', (tester) async { + stubCommunity(buildCommunity()); + stubFeed([ + buildFeedPost( + vote: 'up', + refVoteUri: voteUri, + communityViewer: CommunityRefViewerState(subscribed: true), + ), + ]); + + final auth = _FakeAuthProvider(authenticated: true); + final votes = buildVoteProvider(auth); + final subscriptions = buildSubscriptionProvider(auth); + + await pumpFeedScreen( + tester, + auth: auth, + votes: votes, + subscriptions: subscriptions, + ); + + expect(votes.isLiked(postUri), true); + expect(votes.getVoteState(postUri)?.uri, voteUri); + expect(subscriptions.isSubscribed(communityDid), true); + }); + + testWidgets('C4 the community feed page seeds nothing when signed out, ' + 'on identical input', (tester) async { + stubCommunity(buildCommunity()); + stubFeed([ + buildFeedPost( + vote: 'up', + refVoteUri: voteUri, + communityViewer: CommunityRefViewerState(subscribed: true), + ), + ]); + + final auth = _FakeAuthProvider(authenticated: false); + final votes = buildVoteProvider(auth); + final subscriptions = buildSubscriptionProvider(auth); + + await pumpFeedScreen( + tester, + auth: auth, + votes: votes, + subscriptions: subscriptions, + ); + + expect(votes.isLiked(postUri), false); + expect(votes.getVoteState(postUri), isNull); + expect(subscriptions.isSubscribed(communityDid), false); + }); + + testWidgets('C3 the community feed page skips a post whose community ' + 'viewer says nothing about subscribed', (tester) async { + stubCommunity(buildCommunity()); + stubFeed([ + buildFeedPost(communityViewer: CommunityRefViewerState()), + ]); + + final auth = _FakeAuthProvider(authenticated: true); + final votes = buildVoteProvider(auth); + final subscriptions = buildSubscriptionProvider(auth) + ..setInitialSubscriptionState( + communityDid: communityDid, + isSubscribed: true, + ); + + await pumpFeedScreen( + tester, + auth: auth, + votes: votes, + subscriptions: subscriptions, + ); + + expect(subscriptions.isSubscribed(communityDid), true); + }); + }); +} diff --git a/test/services/viewer_state_hydrator_test.dart b/test/services/viewer_state_hydrator_test.dart new file mode 100644 index 0000000..2e7d3fd --- /dev/null +++ b/test/services/viewer_state_hydrator_test.dart @@ -0,0 +1,284 @@ +// Direct unit tests for ViewerStateHydrator's two gates. +// +// Every method has the same two guards - "is anyone signed in?" and "is the +// target provider wired?" - and dropping either from any one method is a +// silent leak: a signed-out session would adopt the previous account's votes +// and subscriptions, or a null provider would throw mid-fetch. +// +// The call-site tests cover a few of these combinations incidentally. This +// file covers the whole grid on purpose: 7 methods x {signed in, signed out} +// x {provider wired, provider null}. +// +// Asserted with verify / verifyZeroInteractions on mocks, because the +// invariant is about what the hydrator HANDS OVER. What VoteProvider then +// decides to do with a snapshot (adopt it, reconcile it, protect an +// optimistic vote) is VoteProvider's contract and is tested there. + +import 'package:coves_flutter/models/comment.dart'; +import 'package:coves_flutter/models/community.dart'; +import 'package:coves_flutter/models/post.dart'; +import 'package:coves_flutter/providers/community_subscription_provider.dart'; +import 'package:coves_flutter/services/viewer_state_hydrator.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; + +import '../test_helpers/test_mocks.dart'; + +const String postUri = 'at://did:plc:author/social.coves.community.post/p1'; +const String commentUri = 'at://did:plc:author/social.coves.comment/c1'; +const String replyUri = 'at://did:plc:author/social.coves.comment/c2'; +const String communityDid = 'did:plc:community'; +const String voteUri = 'at://did:plc:me/social.coves.feed.vote/v1'; + +/// A subscription provider that records the seeds handed to it. +/// +/// There is no generated mock for CommunitySubscriptionProvider, and its +/// real constructor wants an API client and an auth listener, so this +/// hand-rolled double keeps the test free of both. +class _RecordingSubscriptionProvider implements CommunitySubscriptionProvider { + final List<({String did, bool subscribed})> seeds = []; + + @override + void setInitialSubscriptionState({ + required String communityDid, + required bool isSubscribed, + }) { + seeds.add((did: communityDid, subscribed: isSubscribed)); + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +PostView buildPost() { + return PostView( + uri: postUri, + cid: 'cid-p1', + rkey: 'p1', + author: AuthorView(did: 'did:plc:author', handle: 'test.user'), + community: CommunityRef( + did: communityDid, + name: 'testcove', + viewer: CommunityRefViewerState(subscribed: true), + ), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + record: const PostRecord(title: 'T', content: 'B'), + stats: PostStats(upvotes: 0, downvotes: 0, score: 0, commentCount: 0), + viewer: ViewerState(vote: 'up', voteUri: voteUri), + ); +} + +CommentView buildComment(String uri) { + return CommentView( + uri: uri, + cid: 'cid-$uri', + record: const CommentRecord(content: 'body'), + 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: postUri, cid: 'post-cid'), + stats: const CommentStats(score: 1, upvotes: 1), + viewer: CommentViewerState(vote: 'up', voteUri: voteUri), + ); +} + +CommunityView buildCommunity() { + return CommunityView( + did: communityDid, + name: 'testcove', + viewer: CommunityViewerState(subscribed: true), + ); +} + +void main() { + late MockAuthProvider mockAuthProvider; + late MockVoteProvider mockVoteProvider; + late _RecordingSubscriptionProvider subscriptions; + + setUp(() { + mockAuthProvider = MockAuthProvider(); + mockVoteProvider = MockVoteProvider(); + subscriptions = _RecordingSubscriptionProvider(); + }); + + ViewerStateHydrator buildHydrator({ + required bool authenticated, + bool wired = true, + }) { + when(mockAuthProvider.isAuthenticated).thenReturn(authenticated); + return ViewerStateHydrator( + authProvider: mockAuthProvider, + voteProvider: wired ? mockVoteProvider : null, + subscriptionProvider: wired ? subscriptions : null, + ); + } + + final feed = [FeedViewPost(post: buildPost())]; + final comments = [buildComment(commentUri)]; + final tree = [ + ThreadViewComment( + comment: buildComment(commentUri), + replies: [ThreadViewComment(comment: buildComment(replyUri))], + ), + ]; + final communities = [buildCommunity()]; + + /// Every method, keyed by name, so the grid below stays one line each. + final invocations = { + 'hydrateFeed': (h) => h.hydrateFeed(feed), + 'hydrateFeedVotesOnly': (h) => h.hydrateFeedVotesOnly(feed), + 'hydratePost': (h) => h.hydratePost(buildPost()), + 'hydrateComments': (h) => h.hydrateComments(comments), + 'hydrateCommentTree': (h) => h.hydrateCommentTree(tree), + 'hydrateCommunityListSubscriptions': (h) => + h.hydrateCommunityListSubscriptions(communities), + 'hydrateCommunitySubscription': (h) => + h.hydrateCommunitySubscription(buildCommunity()), + }; + + group('signed out: every method hands over nothing', () { + for (final entry in invocations.entries) { + test('${entry.key} touches neither provider', () { + entry.value(buildHydrator(authenticated: false)); + + // A leak here is another account's viewer state landing in a + // signed-out session. + verifyZeroInteractions(mockVoteProvider); + expect(subscriptions.seeds, isEmpty); + }); + } + }); + + group('provider absent: every method is a no-op rather than a crash', () { + for (final entry in invocations.entries) { + test('${entry.key} does not throw with both providers null', () { + final hydrator = buildHydrator(authenticated: true, wired: false); + + expect(() => entry.value(hydrator), returnsNormally); + verifyZeroInteractions(mockVoteProvider); + expect(subscriptions.seeds, isEmpty); + }); + } + }); + + group('signed in and wired: the positive controls', () { + test('hydrateFeed hands over both the vote and the subscription', () { + buildHydrator(authenticated: true).hydrateFeed(feed); + + verify( + mockVoteProvider.applyServerVoteState( + postUri: postUri, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + expect(subscriptions.seeds, [(did: communityDid, subscribed: true)]); + }); + + test('hydrateFeedVotesOnly hands over the vote and NOT the ' + 'subscription', () { + buildHydrator(authenticated: true).hydrateFeedVotesOnly(feed); + + verify( + mockVoteProvider.applyServerVoteState( + postUri: postUri, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + // Divergence D1, asserted at the source rather than through a caller. + expect(subscriptions.seeds, isEmpty); + }); + + test('hydratePost hands over the single post vote', () { + buildHydrator(authenticated: true).hydratePost(buildPost()); + + verify( + mockVoteProvider.applyServerVoteState( + postUri: postUri, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + expect(subscriptions.seeds, isEmpty); + }); + + test('hydrateComments hands over each flat comment vote', () { + buildHydrator(authenticated: true).hydrateComments(comments); + + verify( + mockVoteProvider.applyServerVoteState( + postUri: commentUri, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + }); + + test('hydrateCommentTree recurses into replies', () { + buildHydrator(authenticated: true).hydrateCommentTree(tree); + + verify( + mockVoteProvider.applyServerVoteState( + postUri: commentUri, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + verify( + mockVoteProvider.applyServerVoteState( + postUri: replyUri, + voteDirection: 'up', + voteUri: voteUri, + ), + ).called(1); + }); + + test('hydrateCommunityListSubscriptions seeds each community', () { + buildHydrator( + authenticated: true, + ).hydrateCommunityListSubscriptions(communities); + + expect(subscriptions.seeds, [(did: communityDid, subscribed: true)]); + verifyZeroInteractions(mockVoteProvider); + }); + + test('hydrateCommunitySubscription seeds the single community', () { + buildHydrator( + authenticated: true, + ).hydrateCommunitySubscription(buildCommunity()); + + expect(subscriptions.seeds, [(did: communityDid, subscribed: true)]); + verifyZeroInteractions(mockVoteProvider); + }); + }); + + group('the null-direction contract', () { + test('a null vote direction is still handed over, not filtered out', () { + // This is how a vote removed on another device gets cleared. A guard + // that skipped nulls would strand the local vote forever. + final post = PostView( + uri: postUri, + cid: 'cid-p1', + rkey: 'p1', + author: AuthorView(did: 'did:plc:author', handle: 'test.user'), + community: CommunityRef(did: communityDid, name: 'testcove'), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + record: const PostRecord(title: 'T', content: 'B'), + stats: PostStats(upvotes: 0, downvotes: 0, score: 0, commentCount: 0), + ); + + buildHydrator(authenticated: true).hydratePost(post); + + verify( + mockVoteProvider.applyServerVoteState( + postUri: postUri, + voteDirection: argThat(isNull, named: 'voteDirection'), + voteUri: argThat(isNull, named: 'voteUri'), + ), + ).called(1); + }); + }); +} diff --git a/test/utils/community_name_validator_test.dart b/test/utils/community_name_validator_test.dart new file mode 100644 index 0000000..f38cdaa --- /dev/null +++ b/test/utils/community_name_validator_test.dart @@ -0,0 +1,224 @@ +// Unit tests for the extracted community-name validator. +// +// Two of these branches could not be reached while the logic lived inside +// the admin panel: the empty-name message (the submit button is disabled +// while any field is blank) and the length-before-charset precedence. They +// had shipped uncovered. +// +// The trap worth stating out loud: the validator NORMALIZES - trims and +// LOWERCASES - before it matches, so an uppercase name is VALID even though +// the charset message talks about lowercase. A validator that rejected +// uppercase would break the panel's happy path. +// +// DELIBERATELY NOT PINNED: the exact wording of the length and charset +// messages. Those are user-facing copy and are expected to be reworded - the +// charset one especially, since it currently promises something the code +// does not enforce. So the tests below identify an error by comparing +// against what the validator ITSELF returns for a known-bad input of that +// class, and anchor each class with a stable fragment plus a +// distinct-from-the-others check. Rewording the copy keeps them green; +// misrouting an input to the wrong branch does not. +// +// The one message asserted literally is 'Name is required', which is the +// precise contract for the empty case and is not slated to change. + +import 'package:coves_flutter/utils/community_name_validator.dart'; +import 'package:flutter_test/flutter_test.dart'; + +const String requiredMessage = 'Name is required'; + +/// One character past the limit, built from the production constant so the +/// cases move automatically if the limit ever does. +String overLongName() => 'a' * (CommunityNameValidator.maxLength + 1); + +String maxLengthName() => 'a' * CommunityNameValidator.maxLength; + +void main() { + // Reference errors, taken from the validator itself rather than copied + // from the source. A branch that stopped firing makes these null and every + // test that uses them fails loudly. + final lengthError = CommunityNameValidator.validate(overLongName()); + final charsetError = CommunityNameValidator.validate('world_news'); + + group('the three error classes are distinct and identifiable', () { + test('each bad input class produces a non-null, distinct message', () { + expect(CommunityNameValidator.validate(''), requiredMessage); + expect(lengthError, isNotNull); + expect(charsetError, isNotNull); + + expect(lengthError, isNot(requiredMessage)); + expect(charsetError, isNot(requiredMessage)); + expect(charsetError, isNot(lengthError)); + }); + + test('the length message names the actual limit', () { + // Asserted on the validator's OWN output, so hardcoding the template + // in this file could not keep it green. + expect(lengthError, contains('${CommunityNameValidator.maxLength}')); + }); + + test('the charset message mentions hyphens and does not name a limit', () { + // A stable fragment: every candidate rewording of this copy still + // has to say which characters are allowed. + expect(charsetError, contains('hyphens')); + expect( + charsetError, + isNot(contains('${CommunityNameValidator.maxLength}')), + ); + }); + }); + + group('normalize', () { + test('trims then lowercases', () { + expect( + CommunityNameValidator.normalize(' MyCommunity '), + 'mycommunity', + ); + }); + + test('leaves an already-normal name alone', () { + expect(CommunityNameValidator.normalize('worldnews'), 'worldnews'); + }); + + test('collapses a whitespace-only name to empty', () { + expect(CommunityNameValidator.normalize(' '), ''); + expect(CommunityNameValidator.normalize('\t\n '), ''); + }); + + test('does not touch interior whitespace', () { + expect(CommunityNameValidator.normalize(' World News '), 'world news'); + }); + }); + + group('validate - required', () { + test('an empty name is required', () { + expect(CommunityNameValidator.validate(''), requiredMessage); + }); + + test('a whitespace-only name is required, not a charset error', () { + // It normalizes to empty, so the required branch wins - this is NOT + // reported as a charset problem. + expect(CommunityNameValidator.validate(' '), requiredMessage); + expect(CommunityNameValidator.validate('\t\n'), requiredMessage); + }); + }); + + group('validate - length', () { + test('a name exactly at the limit is valid', () { + expect(CommunityNameValidator.validate(maxLengthName()), isNull); + }); + + test('one character past the limit is rejected', () { + expect(CommunityNameValidator.validate(overLongName()), lengthError); + }); + + test('length is measured on the TRIMMED name', () { + // Four characters of padding around a name that is exactly at the + // limit: still valid, because trimming happens first. + final padded = ' ${maxLengthName()} '; + expect(padded.length, CommunityNameValidator.maxLength + 4); + expect(CommunityNameValidator.validate(padded), isNull); + }); + + test('length is measured after lowercasing, which cannot change it', () { + expect( + CommunityNameValidator.validate(overLongName().toUpperCase()), + lengthError, + ); + }); + + test('length is measured in UTF-16 code units, not grapheme clusters', () { + // Each emoji is 2 code units, so this is over the limit in units while + // being well under it in characters. Reported as too long rather than + // as a charset problem - and a switch to characters.length would flip + // this, which is exactly why it is pinned. + final emoji = '😀' * CommunityNameValidator.maxLength; + expect(emoji.length, greaterThan(CommunityNameValidator.maxLength)); + expect(CommunityNameValidator.validate(emoji), lengthError); + }); + }); + + group('validate - charset', () { + test('a leading hyphen is rejected', () { + expect(CommunityNameValidator.validate('-worldnews'), charsetError); + }); + + test('a trailing hyphen is rejected', () { + expect(CommunityNameValidator.validate('worldnews-'), charsetError); + }); + + test('a lone hyphen is rejected', () { + expect(CommunityNameValidator.validate('-'), charsetError); + }); + + test('underscores are rejected', () { + expect(CommunityNameValidator.validate('world_news'), charsetError); + }); + + test('interior spaces are rejected', () { + expect(CommunityNameValidator.validate('world news'), charsetError); + }); + + test('dots are rejected', () { + expect(CommunityNameValidator.validate('world.news'), charsetError); + }); + + test('non-ASCII letters are rejected', () { + expect(CommunityNameValidator.validate('wörldnews'), charsetError); + }); + }); + + group('validate - accepted names', () { + test('a plain lowercase name is valid', () { + expect(CommunityNameValidator.validate('worldnews'), isNull); + }); + + test('an UPPERCASE name is VALID, because it normalizes first', () { + // The charset copy talks about lowercase; the code does not enforce + // it. The create request normalizes the same way, so what is sent + // always matches what was validated. + expect(CommunityNameValidator.validate('MyCommunity'), isNull); + expect(CommunityNameValidator.validate('WORLDNEWS'), isNull); + }); + + test('surrounding whitespace is accepted and trimmed away', () { + expect(CommunityNameValidator.validate(' worldnews '), isNull); + }); + + test('a single character is valid', () { + expect(CommunityNameValidator.validate('a'), isNull); + expect(CommunityNameValidator.validate('7'), isNull); + }); + + test('a digits-only name is valid', () { + expect(CommunityNameValidator.validate('2026'), isNull); + }); + + test('interior hyphens are valid', () { + expect(CommunityNameValidator.validate('world-news'), isNull); + expect(CommunityNameValidator.validate('a-b-c-d'), isNull); + // Consecutive interior hyphens are allowed too. + expect(CommunityNameValidator.validate('world--news'), isNull); + }); + }); + + group('validate - precedence between the rules', () { + test('length beats charset when an input violates both', () { + // Over the limit AND containing an underscore: the length rule is + // checked first, so that is the message the user sees. + final tooLongAndBadCharset = '${maxLengthName()}_'; + expect( + tooLongAndBadCharset.length, + greaterThan(CommunityNameValidator.maxLength), + ); + expect( + CommunityNameValidator.validate(tooLongAndBadCharset), + lengthError, + ); + }); + + test('required beats everything for a whitespace-only name', () { + expect(CommunityNameValidator.validate(' ' * 100), requiredMessage); + }); + }); +} diff --git a/test/widgets/post_detail_loader_hydration_test.dart b/test/widgets/post_detail_loader_hydration_test.dart new file mode 100644 index 0000000..bbc6265 --- /dev/null +++ b/test/widgets/post_detail_loader_hydration_test.dart @@ -0,0 +1,199 @@ +// Characterization net for the single-post viewer-state hydration in +// PostDetailLoader (site 6 of the eight hydration sites). +// +// Two properties are pinned here, both through public surfaces only: +// +// * the fresh viewer snapshot is applied when signed in and skipped when +// signed out (positive-control pair on identical input), and +// * the VoteProvider lookup itself happens INSIDE the auth gate. A +// signed-out, VoteProvider-less tree must not blow up in the loader - +// provider-less widget trees rely on that. Moving the lookup out of the +// gate (e.g. resolving a hydrator unconditionally at the top of the +// method) breaks the first assertion below, and the signed-in control +// proves the assertion has teeth. + +import 'package:coves_flutter/models/post.dart'; +import 'package:coves_flutter/models/post_get_result.dart'; +import 'package:coves_flutter/providers/auth_provider.dart'; +import 'package:coves_flutter/providers/vote_provider.dart'; +import 'package:coves_flutter/screens/home/post_detail_loader.dart'; +import 'package:coves_flutter/screens/home/post_detail_screen.dart'; +import 'package:coves_flutter/services/comment_service.dart'; +import 'package:coves_flutter/services/comments_provider_cache.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:coves_flutter/services/vote_service.dart'; +import 'package:coves_flutter/widgets/loading_error_states.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; + +class _FakeAuthProvider extends AuthProvider { + _FakeAuthProvider({required bool authenticated}) + : _isAuthenticated = authenticated; + + final bool _isAuthenticated; + + @override + bool get isAuthenticated => _isAuthenticated; + + @override + bool get isLoading => false; +} + +void main() { + const testUri = 'at://did:plc:test/social.coves.community.post/abc123'; + const coldVoteUri = 'at://did:plc:me/social.coves.feed.vote/cold1'; + + PostView buildPost({ViewerState? viewer}) { + return PostView( + viewer: viewer, + uri: testUri, + cid: 'test-cid', + rkey: 'abc123', + author: AuthorView(did: 'did:plc:author', handle: 'test.user'), + community: CommunityRef( + did: 'did:plc:community', + name: 'test-community', + ), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + record: const PostRecord(content: 'Test body', title: 'Cold Loaded'), + stats: PostStats(score: 42, upvotes: 50, downvotes: 8, commentCount: 5), + ); + } + + VoteProvider buildVoteProvider(AuthProvider auth) { + final provider = VoteProvider( + voteService: VoteService( + sessionGetter: () async => null, + didGetter: () => null, + ), + authProvider: auth, + ); + addTearDown(provider.dispose); + return provider; + } + + /// The full provider set PostDetailScreen needs to build. + Future pumpWithProviders( + WidgetTester tester, { + required AuthProvider auth, + required VoteProvider votes, + }) async { + final apiService = CovesApiService(tokenGetter: () async => null); + addTearDown(apiService.dispose); + + await tester.pumpWidget( + MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: auth), + ChangeNotifierProvider.value(value: votes), + Provider.value( + value: CommentsProviderCache( + authProvider: auth, + voteProvider: votes, + commentService: CommentService(), + apiService: apiService, + ), + ), + ], + child: MaterialApp( + home: PostDetailLoader( + postUri: testUri, + fetchPost: (_) async => PostGetSuccess( + buildPost( + viewer: ViewerState(vote: 'up', voteUri: coldVoteUri), + ), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + } + + testWidgets('C10 applies the fresh post viewer snapshot when signed in', ( + tester, + ) async { + final auth = _FakeAuthProvider(authenticated: true); + final votes = buildVoteProvider(auth); + + await pumpWithProviders(tester, auth: auth, votes: votes); + + expect(find.byType(PostDetailScreen), findsOneWidget); + expect(votes.isLiked(testUri), true); + expect(votes.getVoteState(testUri)?.uri, coldVoteUri); + + await tester.pumpWidget(const MaterialApp(home: Scaffold())); + await tester.pumpAndSettle(); + }); + + testWidgets('C10 applies nothing when signed out, on identical input', ( + tester, + ) async { + final auth = _FakeAuthProvider(authenticated: false); + final votes = buildVoteProvider(auth); + + await pumpWithProviders(tester, auth: auth, votes: votes); + + expect(find.byType(PostDetailScreen), findsOneWidget); + expect(votes.isLiked(testUri), false); + expect(votes.getVoteState(testUri), isNull); + + await tester.pumpWidget(const MaterialApp(home: Scaffold())); + await tester.pumpAndSettle(); + }); + + group('C13 the VoteProvider lookup stays inside the auth gate', () { + /// Pumps the loader with NO Provider in the tree. + /// + /// If the loader resolves the vote surface before checking auth, the + /// resulting ProviderNotFoundException is swallowed by _fetch's broad + /// catch and the loader lands in its terminal error state - which is + /// exactly what the signed-in control below demonstrates. + Future pumpWithoutVoteProvider( + WidgetTester tester, { + required bool authenticated, + }) async { + final auth = _FakeAuthProvider(authenticated: authenticated); + + await tester.pumpWidget( + ChangeNotifierProvider.value( + value: auth, + child: MaterialApp( + home: PostDetailLoader( + postUri: testUri, + fetchPost: (_) async => PostGetSuccess( + buildPost( + viewer: ViewerState(vote: 'up', voteUri: coldVoteUri), + ), + ), + ), + ), + ), + ); + await tester.pump(); + await tester.pump(); + } + + testWidgets('a signed-out provider-less tree gets past hydration', ( + tester, + ) async { + await pumpWithoutVoteProvider(tester, authenticated: false); + + // The loader itself never errored: it handed the post on. (What the + // downstream screen then does with a provider-less tree is its own + // business and is drained below.) + expect(find.byType(FullScreenError), findsNothing); + tester.takeException(); + }); + + testWidgets('the same signed-in tree does error, proving the gate is ' + 'what protects the signed-out case', (tester) async { + await pumpWithoutVoteProvider(tester, authenticated: true); + + expect(find.byType(FullScreenError), findsOneWidget); + tester.takeException(); + }); + }); +}