feat(e20): self-owned ✓ badge on Home + Search cards#584
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds detection and rendering of a "self-owned ✓" badge: presentation UI models gain isCurrentUserOwner; Home/Search/Details/Favourites/Starred viewmodels observe ProfileRepository to set it; RepositoryCard and AppHeader render OfficialBadge; localized ChangesSelf-owned repository badge
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile Summary
Confidence Score: 3/5Merge-blocking P1 bug in search pagination dedup causes the owner badge to permanently show false for page-1 repos when the user is already signed in. One confirmed P1 logic bug drops the ceiling to 4; the bug affects all multi-page search sessions where the user is already signed in (the typical case), pulling the score down to 3. feature/search/presentation/.../SearchViewModel.kt — pagination dedup merge at lines 415–422 must be fixed before merge. Important Files Changed
Sequence DiagramsequenceDiagram
participant PR as ProfileRepository
participant HVM as HomeViewModel
participant SVM as SearchViewModel
participant DVM as DetailsViewModel
participant FVM as FavouritesViewModel
participant SRVM as StarredReposViewModel
participant UI as UI Layer
PR->>HVM: getUser() flow emit
HVM->>HVM: "currentUserLogin = user?.username"
HVM->>HVM: patch isCurrentUserOwner on all loaded repos
HVM-->>UI: "DiscoveryRepositoryUi(isCurrentUserOwner=true/false)"
PR->>SVM: getUser() flow emit
SVM->>SVM: "currentUserLogin = user?.username"
SVM->>SVM: patch isCurrentUserOwner on all loaded repos
Note over SVM: pagination dedup existing.copy() does NOT forward isCurrentUserOwner
SVM-->>UI: "DiscoveryRepositoryUi(isCurrentUserOwner=?)"
PR->>DVM: "combine(getUser(), _state.map{ownerLogin})"
DVM->>DVM: "isOwner = login == ownerLogin (ignoreCase)"
DVM-->>UI: "DetailsState(isCurrentUserOwner=true/false)"
PR->>FVM: combine(getAllFavorites(), getUser())
FVM->>FVM: map with isCurrentUserOwner per repo
FVM-->>UI: "FavouriteRepository(isCurrentUserOwner=true/false)"
PR->>SRVM: combine(getAllStarred(), getAllFavorites(), getUser())
SRVM->>SRVM: map with isCurrentUserOwner per repo
SRVM-->>UI: "StarredRepositoryUi(isCurrentUserOwner=true/false)"
UI->>UI: RepositoryCard / OfficialBadge renders conditionally
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
feature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeViewModel.kt (1)
371-385: ⚡ Quick winConsider caching the current user login to avoid repeated profile lookups.
mapReposToUi(...)callsprofileRepository.getUser().first()every time it maps repositories, which occurs during initial loads, pagination, category switches, and topic supplements. This repeated suspending call could be optimized by caching the current user login once during initialization.♻️ Example caching approach
Add a cached property:
class HomeViewModel( ... private val profileRepository: ProfileRepository, ) : ViewModel() { private var hasLoadedInitialData = false + private var currentUserLogin: String? = null ...Initialize it in
syncSystemState()orloadPlatform():private fun syncSystemState() { viewModelScope.launch { try { + currentUserLogin = profileRepository.getUser().first()?.username val result = syncInstalledAppsUseCase() ...Then use the cached value in
mapReposToUi:-val currentLogin = profileRepository.getUser().first()?.username +val currentLogin = currentUserLogin🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeViewModel.kt` around lines 371 - 385, mapReposToUi currently calls profileRepository.getUser().first() each time which is expensive; add a cached nullable String property (e.g., cachedCurrentLogin) on HomeViewModel, set it once during initialization (e.g., in syncSystemState() or loadPlatform() by calling profileRepository.getUser().first() there) and then update mapReposToUi to use cachedCurrentLogin instead of calling profileRepository.getUser().first(); ensure cachedCurrentLogin is updated whenever user state changes and remove the inline profileRepository.getUser().first() calls in mapReposToUi to avoid repeated suspending lookups.feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt (1)
354-369: ⚡ Quick winConsider caching the current user login to avoid repeated profile lookups.
Every search operation calls
profileRepository.getUser().first()to determine ownership. This suspending call is repeated inappendExploreResults(line 781) as well. If the user performs multiple searches or explores additional pages, this profile lookup happens repeatedly.Consider fetching the current user login once during initialization or when the auth state changes, then reusing the cached value for ownership comparisons throughout the session.
♻️ Example caching approach
Add a cached property:
class SearchViewModel( ... private val profileRepository: ProfileRepository, ) : ViewModel() { private var hasLoadedInitialData = false + private var currentUserLogin: String? = null ...Initialize it in
syncSystemState()or create a dedicated function:private fun syncSystemState() { viewModelScope.launch { try { + currentUserLogin = profileRepository.getUser().first()?.username val result = syncInstalledAppsUseCase() ...Then use the cached value:
-val currentLogin = profileRepository.getUser().first()?.username +val currentLogin = currentUserLogin🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt` around lines 354 - 369, profileRepository.getUser().first() is being called repeatedly to compute currentLogin (used when building DiscoveryRepositoryUi and in appendExploreResults); cache the current username once (e.g., a private var cachedCurrentLogin) during initialization or inside syncSystemState() and update it on auth changes, then replace direct calls to profileRepository.getUser().first() with the cachedCurrentLogin when computing isCurrentUserOwner in the DiscoveryRepositoryUi mapping and in appendExploreResults so the suspending profile lookup is not performed on every search/page load.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@feature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeViewModel.kt`:
- Around line 371-385: mapReposToUi currently calls
profileRepository.getUser().first() each time which is expensive; add a cached
nullable String property (e.g., cachedCurrentLogin) on HomeViewModel, set it
once during initialization (e.g., in syncSystemState() or loadPlatform() by
calling profileRepository.getUser().first() there) and then update mapReposToUi
to use cachedCurrentLogin instead of calling
profileRepository.getUser().first(); ensure cachedCurrentLogin is updated
whenever user state changes and remove the inline
profileRepository.getUser().first() calls in mapReposToUi to avoid repeated
suspending lookups.
In
`@feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt`:
- Around line 354-369: profileRepository.getUser().first() is being called
repeatedly to compute currentLogin (used when building DiscoveryRepositoryUi and
in appendExploreResults); cache the current username once (e.g., a private var
cachedCurrentLogin) during initialization or inside syncSystemState() and update
it on auth changes, then replace direct calls to
profileRepository.getUser().first() with the cachedCurrentLogin when computing
isCurrentUserOwner in the DiscoveryRepositoryUi mapping and in
appendExploreResults so the suspending profile lookup is not performed on every
search/page load.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b56d9024-2a6b-4b1d-b3dd-6fe327305bd7
📒 Files selected for processing (20)
core/presentation/src/commonMain/composeResources/files/whatsnew/17.jsoncore/presentation/src/commonMain/composeResources/values-ar/strings-ar.xmlcore/presentation/src/commonMain/composeResources/values-bn/strings-bn.xmlcore/presentation/src/commonMain/composeResources/values-es/strings-es.xmlcore/presentation/src/commonMain/composeResources/values-fr/strings-fr.xmlcore/presentation/src/commonMain/composeResources/values-hi/strings-hi.xmlcore/presentation/src/commonMain/composeResources/values-it/strings-it.xmlcore/presentation/src/commonMain/composeResources/values-ja/strings-ja.xmlcore/presentation/src/commonMain/composeResources/values-ko/strings-ko.xmlcore/presentation/src/commonMain/composeResources/values-pl/strings-pl.xmlcore/presentation/src/commonMain/composeResources/values-ru/strings-ru.xmlcore/presentation/src/commonMain/composeResources/values-tr/strings-tr.xmlcore/presentation/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcore/presentation/src/commonMain/composeResources/values/strings.xmlcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/RepositoryCard.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/model/DiscoveryRepositoryUi.ktfeature/home/presentation/build.gradle.ktsfeature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeViewModel.ktfeature/search/presentation/build.gradle.ktsfeature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Render a
Verified-icon badge next to the owner login onRepositoryCardwhen the current signed-in user owns the repo. Triggers on Home + Search discovery surfaces. Closes E20 first-slice (Sprint 2 brief, no-backend variant).Plumbing: HomeViewModel + SearchViewModel inject ProfileRepository, read
getUser().first()?.usernameduring repo→UI mapping, set newDiscoveryRepositoryUi.isCurrentUserOwnerflag via case-insensitive login compare.RepositoryCardrendersOfficialBadge(Icons.Filled.Verified, primary tint, 16dp) conditionally next to the owner Text.Tooltip contentDescription =
self_owned_badgestring, localized across 13 locales.Out of scope (follow-up): Details AppHeader, Starred / Favourites / DevProfile surfaces — separate UI models. Will mirror once this lands.
Compile-verified:
:core:presentation+:feature:home:presentation+:feature:search:presentation(Android + JVM) BUILD SUCCESSFUL.Summary by CodeRabbit
New Features
Localization