feat: implement 4 open source contributions - #1613
Conversation
- Add readiness history timeline to ReadinessDrilldown - Add recent searches with localStorage persistence to AssetSearch - Add Hana wallet connector with adapter and network detection - Add switch network button to NetworkMismatchBanner
|
Someone is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@mohammad2226 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughChangesThe PR adds persistent recent asset searches, Hana wallet support with network detection and programmatic network switching, and readiness history fetching and timeline rendering. Associated component tests cover these new behaviors and updated wallet guidance. Asset search
Hana wallet and network switching
Readiness history timeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WalletSelector
participant NetworkMismatchBanner
participant HanaAdapter
participant networkDetection
WalletSelector->>NetworkMismatchBanner: pass detected walletType
NetworkMismatchBanner->>HanaAdapter: switchNetwork(passphrase)
NetworkMismatchBanner->>networkDetection: detectWalletNetwork(walletType)
networkDetection-->>NetworkMismatchBanner: updated network
NetworkMismatchBanner-->>WalletSelector: hide mismatch after success
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
frontend/src/components/NetworkMismatchBanner.test.tsx (1)
107-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the parent dismissal callback.
The banner disappears immediately from local
switchSuccess, so Lines 138-140 pass even if the delayedonDismisscall is broken. AssertonDismissafter advancing the 500 ms delay to cover the state cleanup used byWalletSelector.🤖 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 `@frontend/src/components/NetworkMismatchBanner.test.tsx` around lines 107 - 140, The test around “dismisses banner after a successful simulated network switch” only verifies local banner state, not the delayed parent callback. Advance fake timers by 500 ms after the switch succeeds, then assert that the onDismiss mock was called, while retaining the existing banner disappearance assertion.frontend/src/hooks/useReadinessHistory.ts (1)
32-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing direct unit tests for
useReadinessHistory.Only the presentational
ReadinessDrilldownis tested with hand-supplied props; the hook's fetch/loading/error/abort logic (and theresponse.okhandling above) isn't exercised directly.🤖 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 `@frontend/src/hooks/useReadinessHistory.ts` around lines 32 - 59, Add direct unit tests for useReadinessHistory covering successful fetches, loading state transitions, fetch failures, non-ok response handling through fetchReadinessHistory, and AbortError behavior. Exercise the hook with its real fetch helper or an appropriate mock, and verify history, loading, and error state outcomes rather than only testing ReadinessDrilldown props.frontend/src/components/ReadinessDrilldown.test.tsx (1)
188-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer label-based queries over CSS-class selectors.
Querying by Tailwind utility classes (
span.inline-block.h-2.w-2) couples this test to styling rather than behavior; the dots already carryaria-label="degraded"/"healthy"which Testing Library can query directly.♻️ Proposed refactor
- const dots = document.querySelectorAll('span.inline-block.h-2.w-2') - expect(dots.length).toBe(3) + expect(screen.getAllByLabelText('healthy')).toHaveLength(2) + expect(screen.getAllByLabelText('degraded')).toHaveLength(1)🤖 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 `@frontend/src/components/ReadinessDrilldown.test.tsx` around lines 188 - 189, Update the dot query in the ReadinessDrilldown test to use Testing Library’s accessible label query for the existing “degraded” and “healthy” aria labels instead of the Tailwind class selector, while preserving the assertion that three dots are rendered.
🤖 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.
Inline comments:
In `@frontend/src/components/AssetSearch.tsx`:
- Around line 45-51: Update getRecentSearches to filter parsed entries to
non-empty strings before returning them, excluding nulls and objects that could
break symbol.slice during rendering. Deduplicate the valid values and preserve
the RECENT_SEARCHES_MAX limit on the final returned list.
- Around line 377-401: Update the recent-search rendering and selection logic in
AssetSearch so each recent row is selectable by its symbol without relying on
allResults, including assets whose dynamic API results have been cleared. Align
keyboard navigation and selection with the currently displayed rows, including
recentSearches, so the highlighted row selects the corresponding recent asset
rather than an allResults entry. Add regression coverage for selecting a
dynamically sourced recent asset and selecting a recent row via keyboard.
In `@frontend/src/components/ReadinessDrilldown.tsx`:
- Around line 23-44: Update formatTime and formatDate to explicitly validate the
Date created from ts using its numeric validity before formatting. Return the
original ts when the timestamp is invalid, while preserving the existing
Today/Yesterday and localized formatting behavior for valid dates.
In `@frontend/src/hooks/useReadinessHistory.ts`:
- Around line 14-30: Update fetchReadinessHistory to check response.ok
immediately after fetch and throw or otherwise propagate a descriptive error for
non-2xx responses before content-type handling. Preserve the existing JSON
parsing and entries fallback for successful responses so the hook can surface
failures through historyError instead of treating them as empty history.
In `@frontend/src/utils/networkDetection.ts`:
- Around line 84-87: Update the network detection logic around getNetwork to
recognize direct StellarNetwork identifiers first, then pass any other
string—including a string returned in network.network—through
parseStellarNetworkPassphrase. Ensure raw passphrases are normalized to
testnet/mainnet rather than cast directly, while preserving the existing
networkPassphrase handling.
In `@frontend/src/utils/walletAdapters.ts`:
- Around line 271-277: Update HanaAdapter’s network-switch capability handling:
add a runtime supportsNetworkSwitch check based on window.hana.switchNetwork,
use that capability in the banner instead of the adapter method’s existence, and
make switchNetwork explicitly throw an unsupported error when the capability is
absent rather than resolving successfully.
---
Nitpick comments:
In `@frontend/src/components/NetworkMismatchBanner.test.tsx`:
- Around line 107-140: The test around “dismisses banner after a successful
simulated network switch” only verifies local banner state, not the delayed
parent callback. Advance fake timers by 500 ms after the switch succeeds, then
assert that the onDismiss mock was called, while retaining the existing banner
disappearance assertion.
In `@frontend/src/components/ReadinessDrilldown.test.tsx`:
- Around line 188-189: Update the dot query in the ReadinessDrilldown test to
use Testing Library’s accessible label query for the existing “degraded” and
“healthy” aria labels instead of the Tailwind class selector, while preserving
the assertion that three dots are rendered.
In `@frontend/src/hooks/useReadinessHistory.ts`:
- Around line 32-59: Add direct unit tests for useReadinessHistory covering
successful fetches, loading state transitions, fetch failures, non-ok response
handling through fetchReadinessHistory, and AbortError behavior. Exercise the
hook with its real fetch helper or an appropriate mock, and verify history,
loading, and error state outcomes rather than only testing ReadinessDrilldown
props.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7d4d70d-2afe-4807-9eaa-47648603f167
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
frontend/src/app/walletBoot.tsfrontend/src/components/AssetSearch.test.tsxfrontend/src/components/AssetSearch.tsxfrontend/src/components/NetworkMismatchBanner.test.tsxfrontend/src/components/NetworkMismatchBanner.tsxfrontend/src/components/ReadinessDrilldown.test.tsxfrontend/src/components/ReadinessDrilldown.tsxfrontend/src/components/WalletSelector.test.tsxfrontend/src/components/WalletSelector.tsxfrontend/src/config/api.tsfrontend/src/hooks/useReadinessHistory.tsfrontend/src/utils/networkDetection.tsfrontend/src/utils/walletAdapters.ts
| function getRecentSearches(): string[] { | ||
| try { | ||
| const raw = localStorage.getItem(RECENT_SEARCHES_KEY) | ||
| if (!raw) return [] | ||
| const parsed = JSON.parse(raw) | ||
| if (!Array.isArray(parsed)) return [] | ||
| return parsed.slice(0, RECENT_SEARCHES_MAX) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate individual stored values before rendering.
Array.isArray(parsed) accepts entries such as null or objects, but the dropdown calls symbol.slice(...), causing a render-time crash for malformed localStorage. Filter to non-empty strings before returning; deduplicating here also prevents duplicate persisted rows.
🤖 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 `@frontend/src/components/AssetSearch.tsx` around lines 45 - 51, Update
getRecentSearches to filter parsed entries to non-empty strings before returning
them, excluding nulls and objects that could break symbol.slice during
rendering. Deduplicate the valid values and preserve the RECENT_SEARCHES_MAX
limit on the final returned list.
| ) : !debouncedSearch && recentSearches.length > 0 ? ( | ||
| <div> | ||
| <div className="flex items-center justify-between px-3 py-2 border-b border-gray-100 dark:border-gray-700"> | ||
| <span className="text-xs font-medium text-gray-500 dark:text-gray-400 uppercase tracking-wider"> | ||
| Recent searches | ||
| </span> | ||
| <button | ||
| type="button" | ||
| onClick={handleClearRecentSearches} | ||
| className="text-xs text-blue-600 hover:text-blue-800 dark:text-blue-400 dark:hover:text-blue-300" | ||
| > | ||
| Clear | ||
| </button> | ||
| </div> | ||
| {recentSearches.map((symbol, index) => ( | ||
| <button | ||
| key={symbol} | ||
| id={`asset-result-${index}`} | ||
| role="option" | ||
| aria-selected={false} | ||
| type="button" | ||
| onClick={() => { | ||
| const asset = allResults.find(a => a.symbol === symbol) | ||
| if (asset) handleSelect(asset) | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make recent entries selectable from the same data model used for rendering.
A symbol selected from dynamic API results is persisted, but dynamicResults is cleared once the query clears; allResults.find(...) then returns nothing, so clicking that recent entry does nothing. Keyboard navigation also indexes allResults, not recentSearches, and can select a different asset than the highlighted recent row. Select recents directly by symbol (or persist sufficient asset data) and drive keyboard selection from the displayed rows. Add regressions for a dynamic recent asset and keyboard selection.
🤖 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 `@frontend/src/components/AssetSearch.tsx` around lines 377 - 401, Update the
recent-search rendering and selection logic in AssetSearch so each recent row is
selectable by its symbol without relying on allResults, including assets whose
dynamic API results have been cleared. Align keyboard navigation and selection
with the currently displayed rows, including recentSearches, so the highlighted
row selects the corresponding recent asset rather than an allResults entry. Add
regression coverage for selecting a dynamically sourced recent asset and
selecting a recent row via keyboard.
| function formatTime(ts: string): string { | ||
| try { | ||
| const d = new Date(ts) | ||
| return d.toLocaleTimeString(undefined, { hour: '2-digit', minute: '2-digit' }) | ||
| } catch { | ||
| return ts | ||
| } | ||
| } | ||
|
|
||
| function formatDate(ts: string): string { | ||
| try { | ||
| const d = new Date(ts) | ||
| const today = new Date() | ||
| const yesterday = new Date(today) | ||
| yesterday.setDate(yesterday.getDate() - 1) | ||
| if (d.toDateString() === today.toDateString()) return 'Today' | ||
| if (d.toDateString() === yesterday.toDateString()) return 'Yesterday' | ||
| return d.toLocaleDateString(undefined, { month: 'short', day: 'numeric' }) | ||
| } catch { | ||
| return ts | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fallback for invalid timestamps never triggers.
new Date(ts) doesn't throw on invalid input, and toLocale*String() on an invalid Date returns "Invalid Date" rather than throwing — so the catch fallback to raw ts is dead code, and a malformed entry.timestamp will display "Invalid Date" instead of the original value.
🐛 Proposed fix
function formatTime(ts: string): string {
try {
const d = new Date(ts)
+ if (isNaN(d.getTime())) return ts
return d.toLocaleTimeString(undefined, { hour: '2-digit', minute: '2-digit' })
} catch {
return ts
}
}
function formatDate(ts: string): string {
try {
const d = new Date(ts)
+ if (isNaN(d.getTime())) return ts
const today = new Date()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function formatTime(ts: string): string { | |
| try { | |
| const d = new Date(ts) | |
| return d.toLocaleTimeString(undefined, { hour: '2-digit', minute: '2-digit' }) | |
| } catch { | |
| return ts | |
| } | |
| } | |
| function formatDate(ts: string): string { | |
| try { | |
| const d = new Date(ts) | |
| const today = new Date() | |
| const yesterday = new Date(today) | |
| yesterday.setDate(yesterday.getDate() - 1) | |
| if (d.toDateString() === today.toDateString()) return 'Today' | |
| if (d.toDateString() === yesterday.toDateString()) return 'Yesterday' | |
| return d.toLocaleDateString(undefined, { month: 'short', day: 'numeric' }) | |
| } catch { | |
| return ts | |
| } | |
| } | |
| function formatTime(ts: string): string { | |
| try { | |
| const d = new Date(ts) | |
| if (isNaN(d.getTime())) return ts | |
| return d.toLocaleTimeString(undefined, { hour: '2-digit', minute: '2-digit' }) | |
| } catch { | |
| return ts | |
| } | |
| } | |
| function formatDate(ts: string): string { | |
| try { | |
| const d = new Date(ts) | |
| if (isNaN(d.getTime())) return ts | |
| const today = new Date() | |
| const yesterday = new Date(today) | |
| yesterday.setDate(yesterday.getDate() - 1) | |
| if (d.toDateString() === today.toDateString()) return 'Today' | |
| if (d.toDateString() === yesterday.toDateString()) return 'Yesterday' | |
| return d.toLocaleDateString(undefined, { month: 'short', day: 'numeric' }) | |
| } catch { | |
| return ts | |
| } | |
| } |
🤖 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 `@frontend/src/components/ReadinessDrilldown.tsx` around lines 23 - 44, Update
formatTime and formatDate to explicitly validate the Date created from ts using
its numeric validity before formatting. Return the original ts when the
timestamp is invalid, while preserving the existing Today/Yesterday and
localized formatting behavior for valid dates.
| async function fetchReadinessHistory(signal: AbortSignal): Promise<ReadinessHistoryEntry[]> { | ||
| const base = API_CONFIG.BASE_URL.replace(/\/$/, '') | ||
| const url = `${base}${API_CONFIG.ENDPOINTS.READINESS_HISTORY}` | ||
| const response = await fetch(url, { | ||
| method: 'GET', | ||
| headers: { Accept: 'application/json' }, | ||
| signal, | ||
| mode: 'cors', | ||
| credentials: 'omit', | ||
| }) | ||
| const ct = response.headers.get('content-type') || '' | ||
| if (!ct.includes('application/json')) { | ||
| return [] | ||
| } | ||
| const body: ReadinessHistoryResponse = await response.json() | ||
| return body.entries ?? [] | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
HTTP errors are silently swallowed as "no history".
fetchReadinessHistory never checks response.ok. A non-2xx response (e.g. 500 with a JSON error body, or a proxy error page) falls through to returning [] — the hook then reports loading: false, error: false, history: [] instead of surfacing the failure via error: true, hiding real backend issues from historyError.
🐛 Proposed fix
const response = await fetch(url, {
method: 'GET',
headers: { Accept: 'application/json' },
signal,
mode: 'cors',
credentials: 'omit',
})
+ if (!response.ok) {
+ throw new Error(`Readiness history request failed with status ${response.status}`)
+ }
const ct = response.headers.get('content-type') || ''📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function fetchReadinessHistory(signal: AbortSignal): Promise<ReadinessHistoryEntry[]> { | |
| const base = API_CONFIG.BASE_URL.replace(/\/$/, '') | |
| const url = `${base}${API_CONFIG.ENDPOINTS.READINESS_HISTORY}` | |
| const response = await fetch(url, { | |
| method: 'GET', | |
| headers: { Accept: 'application/json' }, | |
| signal, | |
| mode: 'cors', | |
| credentials: 'omit', | |
| }) | |
| const ct = response.headers.get('content-type') || '' | |
| if (!ct.includes('application/json')) { | |
| return [] | |
| } | |
| const body: ReadinessHistoryResponse = await response.json() | |
| return body.entries ?? [] | |
| } | |
| async function fetchReadinessHistory(signal: AbortSignal): Promise<ReadinessHistoryEntry[]> { | |
| const base = API_CONFIG.BASE_URL.replace(/\/$/, '') | |
| const url = `${base}${API_CONFIG.ENDPOINTS.READINESS_HISTORY}` | |
| const response = await fetch(url, { | |
| method: 'GET', | |
| headers: { Accept: 'application/json' }, | |
| signal, | |
| mode: 'cors', | |
| credentials: 'omit', | |
| }) | |
| if (!response.ok) { | |
| throw new Error(`Readiness history request failed with status ${response.status}`) | |
| } | |
| const ct = response.headers.get('content-type') || '' | |
| if (!ct.includes('application/json')) { | |
| return [] | |
| } | |
| const body: ReadinessHistoryResponse = await response.json() | |
| return body.entries ?? [] | |
| } |
🤖 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 `@frontend/src/hooks/useReadinessHistory.ts` around lines 14 - 30, Update
fetchReadinessHistory to check response.ok immediately after fetch and throw or
otherwise propagate a descriptive error for non-2xx responses before
content-type handling. Preserve the existing JSON parsing and entries fallback
for successful responses so the hook can surface failures through historyError
instead of treating them as empty history.
| const network = await (window.hana as any).getNetwork() | ||
| if (typeof network === 'string') return network.toLowerCase() as StellarNetwork | ||
| if (network?.network) return network.network.toLowerCase() as StellarNetwork | ||
| if (network?.networkPassphrase) return parseStellarNetworkPassphrase(network.networkPassphrase) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Parse passphrases returned as strings.
Line 85 casts any string to StellarNetwork. If Hana returns a raw network passphrase, it becomes a lowercased passphrase instead of testnet/mainnet, causing WalletSelector to reject the wallet as mismatched and the switch flow to fail verification. Recognize direct identifiers first, then pass all other strings—including network.network—through parseStellarNetworkPassphrase.
Proposed fix
- if (typeof network === 'string') return network.toLowerCase() as StellarNetwork
- if (network?.network) return network.network.toLowerCase() as StellarNetwork
+ if (typeof network === 'string') return normalizeHanaNetwork(network)
+ if (network?.network) return normalizeHanaNetwork(network.network)🤖 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 `@frontend/src/utils/networkDetection.ts` around lines 84 - 87, Update the
network detection logic around getNetwork to recognize direct StellarNetwork
identifiers first, then pass any other string—including a string returned in
network.network—through parseStellarNetworkPassphrase. Ensure raw passphrases
are normalized to testnet/mainnet rather than cast directly, while preserving
the existing networkPassphrase handling.
| async switchNetwork(network: string): Promise<void> { | ||
| if (!this.isAvailable()) { | ||
| throw new WalletError('Hana wallet is not installed', 'WALLET_NOT_INSTALLED', this.type) | ||
| } | ||
|
|
||
| try { | ||
| await window.hana!.switchNetwork?.(network) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Expose Hana’s actual switch capability.
Line 277 resolves successfully when window.hana.switchNetwork is absent. Since HanaAdapter always defines switchNetwork, the banner shows a switch CTA instead of manual instructions, then reports a misleading “previous network” error. Add a separate runtime capability check (for example, supportsNetworkSwitch) and use it in the banner; also throw explicitly if switching is invoked unsupported.
🤖 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 `@frontend/src/utils/walletAdapters.ts` around lines 271 - 277, Update
HanaAdapter’s network-switch capability handling: add a runtime
supportsNetworkSwitch check based on window.hana.switchNetwork, use that
capability in the banner instead of the adapter method’s existence, and make
switchNetwork explicitly throw an unsupported error when the capability is
absent rather than resolving successfully.
2 participants
closes #1451
closes #1450
closes #1461
closes #1462
Summary by CodeRabbit
New Features
Bug Fixes
Tests