feat: add split detail repository with typed contract#553
Closed
jotel-dev wants to merge 2 commits into
Closed
Conversation
- Create splitDetailRepository.ts with typed contract for split detail data - Add session.ts utility for participant directory fallbacks - Add api-client.ts refactored from api.ts - Create SplitDetailPage.tsx consuming normalized view model - Add repository tests for edge cases (missing profiles, receipts, activity failures) - Page no longer coordinates raw API fan-out or session fallback directly
|
@jotel-dev is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds session-based participant management and a new split detail service layer to the frontend. Introduces a SplitDetailPage component that fetches and displays split information with error handling and loading states. Also adds API client utilities with authentication interceptors and modifies Soroban contract query/storage accessors. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as SplitDetailPage
participant Repo as splitDetailRepository
participant API as API Client
participant Session as sessionManager
participant Storage as localStorage
User->>UI: Load split detail page
activate UI
UI->>Repo: getSplitDetail(splitId, options)
activate Repo
Repo->>API: Fetch split + activities + receipts (concurrent)
activate API
API->>Storage: Check auth token
API-->>Repo: Return split, activities, receipts
deactivate API
Repo->>Session: Load participant directory
activate Session
Session->>Storage: Load tiptune_participant_directory
Session-->>Repo: Participant records
deactivate Session
Repo->>API: Fetch artist profiles for collaborators
activate API
API-->>Repo: Profile data
deactivate API
Repo->>Session: Upsert resolved profiles
activate Session
Session->>Storage: Persist participant directory
Session-->>Repo: Confirmation
deactivate Session
Repo->>Repo: Hydrate activities with actorName
Repo->>Repo: Compute meta flags (missing profiles, fetch failures)
Repo-->>UI: SplitDetailViewModel
deactivate Repo
alt Success
UI->>UI: Render split overview, collaborators, activities, receipts
UI-->>User: Display split details
else Error
UI->>UI: Show error card with retry/back actions
UI-->>User: Display error state
end
deactivate UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/auto-royalty-distribution/src/storage.rs`:
- Around line 91-95: There are two duplicate get_global_log_count definitions
and the real get_distribution_log body was accidentally split into orphaned
statements; remove the duplicate get_global_log_count (the second definition)
and restore/implement get_distribution_log by moving the orphaned statements
into a function named get_distribution_log(env: &Env, idx: u64) ->
Option<DistributionLog> (or the original signature used by callers), using
extend_instance(env) and
env.storage().instance().get(&StorageKey::DistributionLogs(idx)) to fetch the
log, and keep the original get_global_log_count implementation that calls
env.storage().instance().get(&StorageKey::GlobalLogCount).unwrap_or(0); ensure
symbols match callers in queries.rs and lib.rs.
In `@frontend/src/services/splitDetailRepository.test.ts`:
- Around line 21-25: Replace the call to vi.clearAllMocks() in the beforeEach
block with vi.resetAllMocks() to ensure mock implementations are reset between
tests; specifically update the beforeEach that currently references
vi.clearAllMocks() and the sessionManager participantDirectory reset so that
tests like "should handle missing receipts" no longer leak prior mock
implementations.
In `@frontend/src/services/splitDetailRepository.ts`:
- Around line 162-163: The code currently calls
resolveParticipantNames(split.collaborators, profiles) and also runs the
session-hydration block regardless of useSessionFallback; make both respect the
useSessionFallback flag by gating them: either wrap the call to
resolveParticipantNames in an if (useSessionFallback) { ... } or change
resolveParticipantNames to accept a useSessionFallback boolean and skip any
session-based lookups when false, and similarly guard the session hydration
logic that currently runs (the block hydrating profiles from session) so it only
executes when useSessionFallback is true; update callers accordingly to ensure
no session data is read when useSessionFallback is false.
- Around line 157-160: The computed hasMissingProfiles is wrong because it only
checks values present in profiles and therefore misses collaborator IDs that
aren't keys in the profiles map; update the check to iterate the source
collaborator list (split.collaborators) and detect any id that is missing or
falsy in the hydrateProfiles result (e.g., change the predicate to check
split.collaborators.some(collabId => !profiles[collabId]) or otherwise compare
collaborator IDs against Object.keys(profiles)) so missing collaborator entries
are detected; update the variable used in the surrounding logic accordingly and
keep referencing hydrateProfiles and split.collaborators.
In `@frontend/src/utils/api-client.ts`:
- Around line 32-36: The 401 handler in frontend/src/utils/api-client.ts
directly references browser globals (localStorage and window) which will throw
in non-browser runtimes; wrap the redirect/remove logic in a runtime guard such
as if (typeof window !== 'undefined' && typeof localStorage !== 'undefined') {
localStorage.removeItem('authToken'); window.location.href = '/login'; } so the
code in the 401 branch of your error handler (the block checking
error.response?.status === 401) only performs the browser redirect when globals
exist, and otherwise simply lets the error continue to propagate.
In `@frontend/src/utils/session.ts`:
- Around line 87-90: The clear() method calls
localStorage.removeItem(this.storageKey) without the same protection used
elsewhere; wrap the removeItem call in the same try/catch (or guard helper) used
by other storage methods so storage failures in restricted contexts are
swallowed and logged instead of throwing, keeping
this.participantDirectory.clear() behavior unchanged and using this.storageKey
in the safe call.
🪄 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
Run ID: ae2d34ef-e468-4bbd-9ece-f964c1b27b4a
📒 Files selected for processing (7)
contracts/auto-royalty-distribution/src/queries.rscontracts/auto-royalty-distribution/src/storage.rsfrontend/src/pages/SplitView/SplitDetailPage.tsxfrontend/src/services/splitDetailRepository.test.tsfrontend/src/services/splitDetailRepository.tsfrontend/src/utils/api-client.tsfrontend/src/utils/session.ts
Comment on lines
+162
to
+163
| const participantNamesResolved = this.resolveParticipantNames(split.collaborators, profiles); | ||
|
|
There was a problem hiding this comment.
useSessionFallback is bypassed in two paths.
Even when useSessionFallback is false, Line 162 still resolves names from session, and Lines 323-332 still hydrate from session. This makes the option unreliable.
Suggested fix
- const participantNamesResolved = this.resolveParticipantNames(split.collaborators, profiles);
+ const participantNamesResolved = useSessionFallback
+ ? this.resolveParticipantNames(split.collaborators, profiles)
+ : false;- } else {
+ } else if (useSessionFallback) {
// Use session fallback as last resort
const sessionParticipant = sessionManager.getParticipant(collab.artistId);
if (sessionParticipant) {
profiles[collab.artistId] = {
id: sessionParticipant.id,
name: sessionParticipant.name,
avatar: sessionParticipant.avatar,
walletAddress: sessionParticipant.walletAddress,
};
}
}Also applies to: 323-332
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/services/splitDetailRepository.ts` around lines 162 - 163, The
code currently calls resolveParticipantNames(split.collaborators, profiles) and
also runs the session-hydration block regardless of useSessionFallback; make
both respect the useSessionFallback flag by gating them: either wrap the call to
resolveParticipantNames in an if (useSessionFallback) { ... } or change
resolveParticipantNames to accept a useSessionFallback boolean and skip any
session-based lookups when false, and similarly guard the session hydration
logic that currently runs (the block hydrating profiles from session) so it only
executes when useSessionFallback is true; update callers accordingly to ensure
no session data is read when useSessionFallback is false.
Comment on lines
+32
to
+36
| if (error.response?.status === 401) { | ||
| // Handle unauthorized access | ||
| localStorage.removeItem('authToken'); | ||
| window.location.href = '/login'; | ||
| } |
There was a problem hiding this comment.
Guard browser-only redirect logic in the 401 handler.
Line 35 assumes window is always available. If this client is ever exercised outside a browser runtime, this path will throw before the original error is propagated.
Suggested fix
if (error.response?.status === 401) {
// Handle unauthorized access
localStorage.removeItem('authToken');
- window.location.href = '/login';
+ if (typeof window !== 'undefined') {
+ window.location.href = '/login';
+ }
}📝 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.
Suggested change
| if (error.response?.status === 401) { | |
| // Handle unauthorized access | |
| localStorage.removeItem('authToken'); | |
| window.location.href = '/login'; | |
| } | |
| if (error.response?.status === 401) { | |
| // Handle unauthorized access | |
| localStorage.removeItem('authToken'); | |
| if (typeof window !== 'undefined') { | |
| window.location.href = '/login'; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/utils/api-client.ts` around lines 32 - 36, The 401 handler in
frontend/src/utils/api-client.ts directly references browser globals
(localStorage and window) which will throw in non-browser runtimes; wrap the
redirect/remove logic in a runtime guard such as if (typeof window !==
'undefined' && typeof localStorage !== 'undefined') {
localStorage.removeItem('authToken'); window.location.href = '/login'; } so the
code in the 401 branch of your error handler (the block checking
error.response?.status === 401) only performs the browser redirect when globals
exist, and otherwise simply lets the error continue to propagate.
Comment on lines
+87
to
+90
| clear(): void { | ||
| this.participantDirectory.clear(); | ||
| localStorage.removeItem(this.storageKey); | ||
| } |
There was a problem hiding this comment.
Handle clear() storage failures consistently.
Line 89 calls localStorage.removeItem without a guard, while other storage methods are protected. In restricted storage contexts this can throw and break the caller path.
Suggested fix
clear(): void {
- this.participantDirectory.clear();
- localStorage.removeItem(this.storageKey);
+ this.participantDirectory.clear();
+ try {
+ localStorage.removeItem(this.storageKey);
+ } catch (error) {
+ console.warn('Failed to clear participant directory from storage:', error);
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/utils/session.ts` around lines 87 - 90, The clear() method calls
localStorage.removeItem(this.storageKey) without the same protection used
elsewhere; wrap the removeItem call in the same try/catch (or guard helper) used
by other storage methods so storage failures in restricted contexts are
swallowed and logged instead of throwing, keeping
this.participantDirectory.clear() behavior unchanged and using this.storageKey
in the safe call.
- Remove unused imports from splitDetailRepository.ts - Replace 'any' types with proper inline types for API responses - Fix error type in catch blocks to use 'unknown' - Fix type cast in test file for sessionManager mock - Remove duplicate type exports
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.
closes #462