Skip to content

feat(ui): Archived Discussions screen + ViewModel + Settings entry (#94)#115

Merged
ilmoniemi merged 4 commits into
mainfrom
feature/94
May 14, 2026
Merged

feat(ui): Archived Discussions screen + ViewModel + Settings entry (#94)#115
ilmoniemi merged 4 commits into
mainfrom
feature/94

Conversation

@ilmoniemi
Copy link
Copy Markdown
Contributor

What

  • New ArchivedDiscussionsScreen + ArchivedDiscussionsViewModel under ui/settings/. MVI shape per CLAUDE.md: sealed UiState (Loading | Empty | Loaded | Error) and sealed Event (RestoreRequested | BackTapped).
  • Settings → Storage row "Archived discussions" navigates to the new screen (replaces the previous placeholder "Archived conversations / 11 archived" row).
  • New nav route Routes.ARCHIVED_DISCUSSIONS wired in MainActivity.PyryNavHost.
  • Koin: one viewModel { ArchivedDiscussionsViewModel(get()) } line in AppModule.
  • Strings: archived_discussions_title, archived_discussions_empty, archived_discussions_settings_row, restore_action. Reused cd_back.

VM consumes repository.observeConversations(ConversationFilter.Archived) and applies a !isPromoted post-filter (so archived channels are excluded; only archived discussions show). RestoreRequested(id) fires viewModelScope.launch { repository.unarchive(id) } — the row drops naturally from the next upstream emission.

Screen reuses the existing ConversationRow with Modifier.alpha(0.65f) for the muted/archived treatment (mirrors the discussion-row treatment in DiscussionListScreen). Long-press → DropdownMenu with one "Restore" item — exactly one gesture per ticket guidance.

Issue

Closes #94

Blocked-by #93 and #96 are both merged; the archived stream and unarchive primitive are live.

Testing

  • ./gradlew test — passes. New ArchivedDiscussionsViewModelTest covers: initial Loading, Archived filter is requested, post-filter drops promoted-archived, Empty (empty list + only-channels), RestoreRequested calls unarchive with the right id, BackTapped is a no-op for the repo, stream-error → Error, null/blank exception message → non-blank fallback.
  • ./gradlew lint — clean.
  • ./gradlew assembleDebug — succeeds.
  • ./gradlew connectedAndroidTest — not run (no device/emulator in this dev env).

Architecture compliance

  • Stateless screen, state hoisted to the VM; state: UiState, onEvent: (Event) -> Unit shape.
  • VM exposure: StateFlow<UiState> via .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000L), Loading) — mirrors DiscussionListViewModel.
  • No swipe gesture (spec: exactly one gesture). Long-press dropdown matches DiscussionListScreen's convention.
  • BackTapped is Unit in the VM; back navigation handled by NavHost (matches DiscussionListEvent.BackTapped).
  • No optimistic state on restore: repository's MutableStateFlow.update re-emits and the filter drops the row.

Open items (per spec, deferred)

  • archivedAt timestamp field on Conversation is a follow-up (separate ticket when the 30-day auto-archive worker lands). For now the trailing slot renders formatRelativeTime(lastUsedAt), which for the seeded archived discussion prints "Apr 15, 2026" — acceptable as the "archive date" surface for this slice.
  • Settings row no longer shows a live count of archived items (the previous "11 archived" was placeholder copy). Adding a real count expands scope into another SettingsViewModel StateFlow and is deferred per the spec.

Note on spotless

./gradlew spotlessCheck reports one pre-existing violation in app/src/main/java/de/pyryco/mobile/ui/settings/ThemePickerDialog.kt:48 (chained-call line-break) that exists on main and is not in this diff. Per scope discipline (CLAUDE.md "Bug Found Out of Scope") I left it alone — happy to file as a separate cleanup ticket on request.

🤖 Generated with Claude Code

ilmoniemi and others added 2 commits May 14, 2026 09:47
Adds an "Archived discussions" entry under Settings → Storage that
navigates to a new screen listing archived (isPromoted = false,
archived = true) conversations observed from ConversationRepository.
Long-press a row to reveal "Restore", which invokes
ConversationRepository.unarchive(id) and drops the row from the next
emission.

Closes #94

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi
Copy link
Copy Markdown
Contributor Author

Code Review: #94

Decision: FAIL

Findings

  • [MUST FIX] app/src/androidTest/java/de/pyryco/mobile/ui/settings/SettingsScreenTest.kt:25,44SettingsScreen(...) is invoked without the new required onOpenArchivedDiscussions parameter. ./gradlew :app:compileDebugAndroidTestKotlin fails with:
    e: SettingsScreenTest.kt:29:21 No value passed for parameter 'onOpenArchivedDiscussions'.
    e: SettingsScreenTest.kt:48:21 No value passed for parameter 'onOpenArchivedDiscussions'.
    
    The PR's "Testing" section notes connectedAndroidTest was skipped, which is why this slipped through — but the instrumented test sources also won't compile, so any future connectedAndroidTest run on this branch will fail before it boots an emulator. Fix: add onOpenArchivedDiscussions = {} to both call sites (matches the preview-level treatment in SettingsScreen.kt:297,311). A grep / codegraph_callers SettingsScreen against the pre-change shape would have caught this — please run that check on any future signature change.

Other observations (none blocking)

  • VM, screen, navigation wiring, Koin entry, strings, and tests all match the spec verbatim — package layout, WhileSubscribed(5_000L), !isPromoted post-filter, BackTapped → Unit in VM with NavHost interception, Modifier.alpha(0.65f) muted treatment, long-press DropdownMenu (one gesture, not both).
  • Unit tests are thorough: filter capture, post-filter for archived-channels, both Empty paths, RestoreRequested → unarchive(id), BackTapped no-op, stream error, null/blank exception message fallback. Covers the AC's "ViewModel unit test covers the archived-list observation and the restore-action path."
  • ./gradlew test, ./gradlew lint, ./gradlew assembleDebug are clean. The pre-existing spotlessCheck violation in ThemePickerDialog.kt:48 is on main, not in this diff — agreed it's out of scope for this ticket.
  • M3 token usage clean; no hardcoded colors, typography, or shapes. The alpha(0.65f) constant mirrors DiscussionListScreen's established pattern.

Summary

One blast-radius miss: the new required onOpenArchivedDiscussions parameter on SettingsScreen wasn't propagated to SettingsScreenTest.kt, breaking compileDebugAndroidTestKotlin. Everything else in the PR is solid and faithful to the spec. Add the two onOpenArchivedDiscussions = {} arguments to the test file and re-run ./gradlew :app:compileDebugAndroidTestKotlin to confirm before re-review.

…Test

Fixes the compileDebugAndroidTestKotlin failure flagged by code review on
#115: the new required SettingsScreen parameter wasn't propagated to the
two instrumented-test setContent call sites.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi
Copy link
Copy Markdown
Contributor Author

Code Review: #94

Decision: PASS

Findings

  • [NIT] ArchivedDiscussionsScreen.kt:97ConversationRow(onClick = {}) still produces a ripple on tap even though tap is a no-op. Same precedent in DiscussionListScreen.kt:200 (onClick = onTap is the live nav, not a no-op), so this is a deviation only here. Not blocking; if you want, swap to a non-clickable variant in a future cleanup. Long-press-as-sole-action is consistent with the spec and the existing pattern, so leaving it as-is is fine for this slice.
  • [NIT] ArchivedDiscussionsScreen.kt:60,68"Loading…" and "Couldn't load archived discussions: ${state.message}" are hard-coded strings rather than stringResource. Same pattern is in DiscussionListScreen.kt:89,97, so this is codebase-consistent — flagging only because a future strings-pass should sweep both.
  • [NIT] The "last-message preview" AC item maps to an empty supporting-text slot (lastMessage = null). Documented in the spec's open questions as a Phase 1 deferral; acceptable.

Summary

Implementation matches the spec verbatim: VM uses observeConversations(ConversationFilter.Archived) + !isPromoted post-filter, .catchError(message) with non-blank fallback, .stateIn(WhileSubscribed(5_000L), Loading). Restore is a fire-and-forget viewModelScope.launch { repository.unarchive(id) }. Screen mirrors DiscussionListScreen shape (Scaffold + TopAppBar + back arrow + LazyColumn + long-press DropdownMenu + alpha(0.65f) muted row). Settings row updated in place; nav graph extends with Routes.ARCHIVED_DISCUSSIONS; Koin wiring is one line. Strings extracted to strings.xml. SettingsScreenTest propagates the new callback.

VM tests cover all the spec's required scenarios (Loading initial, Archived filter passed, archived-channels dropped, empty source, all-channels-empty, restore-id captured, back-doesn't-dispatch, error path with raw message, non-blank fallback). Three @Preview variants (Loaded/Empty/Long-press menu) provided.

./gradlew :app:testDebugUnitTest and :app:lintDebug both green. Manual verification of the seeded archived discussion + dark mode is the developer's responsibility (per spec § Manual verification).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi ilmoniemi merged commit 6412d05 into main May 14, 2026
1 check failed
@ilmoniemi ilmoniemi deleted the feature/94 branch May 14, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(ui): Archived Discussions screen + ViewModel + Settings entry

1 participant