Add get_campaign_stats query for campaign detail page (#538)#575
Add get_campaign_stats query for campaign detail page (#538)#575Emelie-Dev wants to merge 33 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@Emelie-Dev 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! 🚀 |
davidmaronio
left a comment
There was a problem hiding this comment.
the feature itself (CampaignStats type, get_campaign_stats query, top_contributor / last_contribution_time tracking) is reasonable and reuses MaybePendingCreator. but one change makes this unmergeable as money code:
- src/storage.rs (~397-482) rewrites the entire DataKey encoding, dropping #[contracttype] and hand-rolling IntoVal with numeric discriminants for all ~50 variants. that changes the on-chain serialization of every existing key (Admin, Token, Campaign, Contribution, votes, reserves). on any deployed instance this orphans all existing state. it is unnecessary for a stats query, add the 3 new variants to the existing enum instead and revert the encoding rewrite.
- stats correctness: top_contributor is only cleared when the top contributor themselves refunds (contributions.rs:174), leaving stale data otherwise, and the >= lets a later equal contributor overwrite the top. tighten that.
- confirm the verify_milestone change actually writes the mutated milestones vec back to storage; iterating a non-mut copy would silently no-op.
drop the DataKey rewrite and this is close.
Co-authored-by: Cursor <cursoragent@cursor.com>
…melie-Dev/ProofOfHeart-stellar into feature/538-get-campaign-stats
davidmaronio
left a comment
There was a problem hiding this comment.
thanks for pushing an update, but the blocking issue is still here so i can't clear it yet. the new head still splits the single DataKey enum into four #[contracttype] enums (DataKey/CampaignKey/GovernanceKey/CategoryKey in storage.rs, re-exported in lib.rs:190). moving variants like Campaign(u32)/Contribution/RevenuePool/ApproveVotes/CategoryCampaigns into new enums changes the contracttype key encoding for every existing key, which orphans all existing on-chain campaign/contribution/vote state, that's the same migration we flagged before.
the get_campaign_stats query itself is a fine read, the problem is it's bundled with the storage rewrite. i get that the 52-variant enum hitting LengthExceedsMax is a real constraint, but the split isn't a non-breaking way to solve it. can you land get_campaign_stats on top of the current DataKey layout (no enum restructuring), and if the length limit is genuinely blocking, let's discuss a proper migration path separately? clippy + test are also red on the current head.
|
saw your latest push (Fixed Issues), test + fmt are green now, good. but the core blocker is still open: storage.rs still splits DataKey into CampaignKey/GovernanceKey/CategoryKey, which is the change that re-encodes existing on-chain keys and orphans current state, and clippy is still red. the test fixes don't address that. to unblock: keep get_campaign_stats on the existing single DataKey layout (no enum split), and clear the clippy failure (check the run log). ping me once that's done and i'll re-review right away. |
|
Fixed the clippy failures — two dead imports ( On the migration concern: no keys are orphaned. Soroban encodes contracttype enum variants as Also, the split is required to compile at all — a single 52-variant enum panics with Ready to merge from my side — happy to walk through the XDR diff if you want to verify directly. |
|
thanks, i can see fmt/clippy/test are all green now, appreciate you chasing those down. but the thing that's actually blocking this isn't CI, it's the storage layout: storage.rs still splits the single DataKey enum into DataKey/CampaignKey/GovernanceKey/CategoryKey. moving variants into new #[contracttype] enums changes the XDR key encoding, so any campaign/contribution/vote state already written under the old DataKey::... keys becomes unreachable after upgrade. green tests don't cover that because the test env starts from empty storage. so two paths:
|
|
Good question — I don't have visibility into deployment history since I'm working from a fork. Has this contract been deployed anywhere with real campaign/contribution/vote state (testnet with active testers, mainnet, etc.), or has every deploy so far been fresh for development/testing? Let me know and I'll act on whichever path applies. |
closes #538