[#503] Profile page — reader portfolio and token holdings#508
[#503] Profile page — reader portfolio and token holdings#508realproject7 merged 4 commits intomainfrom
Conversation
- Query on-chain token balances via balanceOf multicall for all storyline tokens, filtered to non-zero holdings - Per-token metrics: story title, token balance, current value (from priceForNextMint), entry price (from first mint in trade_history), last traded date, genre - Total portfolio value in $PLOT equivalent - Sorted by most recently traded, then largest position - Donation history: donations given as reader with totals - Enhanced empty state for addresses with no holdings Fixes #503 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The holdings list and portfolio value look close, but the Portfolio tab still omits one of the issue's required data points: aggregate donations received as writer.
Findings
- [medium]
PortfolioTabonly queriesdonationswheredonor_address = address, so it implements donations given but never surfaces the required aggregate donations received as writer. BecausehasDonationsis based only on that reader-side query, a writer who has received donations but has no holdings and has never donated will incorrectly see the empty state even though the ticket requires writer donation totals in this tab.- File:
src/app/profile/[address]/page.tsx:650 - File:
src/app/profile/[address]/page.tsx:674 - Suggestion: Add a writer-side donations-received aggregate for the profiled address's storylines and include it in the Portfolio tab/empty-state logic.
- File:
Decision
Requesting changes because issue #503 explicitly requires donation history to include donations given as reader plus aggregate donations received as writer, and the current implementation only covers the reader side.
Instead of fetching all storylines and multicalling balanceOf for every token, first query trade_history for storylines the user has actually traded, then scope the balanceOf multicall to only those tokens. This keeps the query bounded as storyline count grows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The scoped holdings multicall fix looks fine, but the Portfolio tab still omits the issue's required aggregate donations received as writer.
Findings
- [medium]
PortfolioTabstill only queriesdonationswheredonor_address = address, so it covers donations given but never surfaces the required writer-side donations received aggregate. BecausehasDonationsstill depends only on that reader-side query, a writer who has received donations but has no holdings and has never donated will still hit the empty state.- File:
src/app/profile/[address]/page.tsx:661 - File:
src/app/profile/[address]/page.tsx:683 - Suggestion: Add a donations-received aggregate for the profiled address's storylines and include it in the Portfolio tab plus empty-state logic.
- File:
Decision
Requesting changes because issue #503 requires donation history to include donations given as reader plus aggregate donations received as writer, and the current implementation still only covers the reader side.
- Query storylines written by address, then sum donations received across all of them - Show "Donations Received" summary card with total amount and count - Empty state now considers donations received (writer who received donations but has no holdings no longer hits empty state) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The writer-donations aggregate is fixed, but the holdings query is now too narrowly scoped: it only checks tokens for storylines present in the address's trade_history. That means the Portfolio tab can miss real holdings and therefore does not satisfy the ticket's requirement to show all storyline tokens the address holds.
Findings
- [high]
PortfolioTabonly multicallsbalanceOffor storylines that already appear intrade_historyfor the address. The issue requires showing all storyline tokens the address currently holds, and the existing reader portfolio pattern scans all storylines before filtering by on-chain balance. The new bounded query will miss holdings acquired outside the indexed trade path (for example transfers or any other source not represented in that address-scopedtrade_history).- File:
src/app/profile/[address]/page.tsx:561 - File:
src/components/ReaderPortfolio.tsx:29 - Suggestion: Follow the existing
ReaderPortfoliopattern here: scan all eligible storylines, then filter bybalanceOf > 0, rather than prefiltering bytrade_historyownership assumptions.
- File:
Decision
Requesting changes because the current implementation can omit real token holdings and therefore does not meet issue #503's core portfolio requirement.
Revert to scanning all non-hidden storylines with tokens (matching the existing ReaderPortfolio pattern) to catch holdings acquired via direct transfers or other paths outside the indexed trade history. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The portfolio tab now covers the required reader holdings, portfolio value, donations given, and aggregate donations received as writer. The holdings query again matches the existing reader-portfolio pattern, so transferred or otherwise externally acquired holdings are no longer missed.
Findings
- None.
Decision
Approve because the portfolio tab now satisfies issue #503's core requirements and my prior requested change is resolved.
Summary
balanceOfmulticallpriceForNextMint), entry price (from first mint in trade_history), last traded date, genre$PLOTequivalentTest plan
Fixes #503
🤖 Generated with Claude Code