Repository navigation
Fix chart empty states, prediction note sanitization, and reconciliation/retention coverage - #1910
Merged
Olowodarey merged 4 commits intoSep 28, 2026
Conversation
Guard interactive-chart against misleading empty axes by adding an explicit loading skeleton and a no-data empty state, reusing the existing Skeleton and EmptyState components. Also guards scaling against single-point series and all-zero values, and adds a visually hidden accessible summary of each series for screen readers. Closes Arena1X#1578
Strip HTML/script markup and control characters from prediction notes before persisting, and reject a sanitized note that is still over the 1000 character limit with a clear NOTE_TOO_LONG validation error. Enforcement lives in PredictionsService.updateNote (defense in depth alongside the DTO's own MaxLength/Transform), since the app has no global ValidationPipe wired up and this method is also called directly elsewhere. Closes Arena1X#1624
Add dedicated tests for AnalyticsService.getRetention confirming a cohort with signups but zero return visits reports 0% (not NaN), a period with zero signups returns a defined empty result instead of throwing, and a cohort where every user returned reports 100%. The existing totalUsers > 0 guard in computeRetention already produces this behavior; this closes the missing coverage called out in the issue and pins the behavior against regressions. Closes Arena1X#1855
Extend reconcileSeasonDistribution to return the ledger rows still missing a confirmed (SUCCEEDED) payout, so a partial payout failure is identified precisely instead of re-flagging the whole season as undistributed. A season whose ledger is fully SUCCEEDED and matches the pool now reports an empty missingRecipients list, a no-op needing no further action. Existing per-recipient ledger idempotency in computeSeasonRewards already prevents re-paying a succeeded recipient on rerun; added a test exercising that path through reconciliation directly. Closes Arena1X#1854
|
@ElizabethChi 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! 🚀 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch had an error being deployed
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
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.
Summary
This batches four assigned issues.
#1578: Interactive chart empty and loading states
interactive-chart.tsxhad no loading or empty-data handling and rendered a misleading empty plot. Added an explicit loading skeleton (reusing the existingSkeletoncomponent), an empty state for no-data series (reusing the existingEmptyStatecomponent), a guard against single-point/all-zero series breaking percentage-based scaling, and a visually hidden accessible summary of each series for screen readers.Closes #1578
#1624: Prediction note length and sanitization
UpdatePredictionNoteDto.notealready had@MaxLength(1000), but nothing sanitized the content before it was persisted, and there is no globalValidationPipewired up inmain.ts, so DTO decorators alone are not a reliable enforcement point. Added asanitizePlainTextutility (strips HTML/script markup and control characters) and enforced it directly inPredictionsService.updateNote, throwing a newNoteTooLongException(NOTE_TOO_LONG, 400) when the sanitized note is still over the limit. The DTO also gets a@Transformusing the same utility as a first line of defense for any call path that does go through a validation pipe.I deliberately left the
notecolumn's migration/type as-is (unboundedtext) since the issue's ask is about API-layer bounding and sanitization; changing the column to a bounded type would be a separate, higher-risk schema change.Closes #1624
#1854: Season distribution reconciliation after partial payout failure
reconcileSeasonDistributiononly summedSUCCEEDEDledger rows against the pool total and returned a boolean match. Extended it to also returnmissingRecipients: the ledger rows that are not yetSUCCEEDED(i.e. stillPENDINGorFAILED). A partial failure now identifies exactly which recipients are still owed instead of only reporting a pool-level mismatch, and a season whose ledger is fullySUCCEEDEDreports an emptymissingRecipientslist (no-op). The existing per-recipient ledger idempotency incomputeSeasonRewards(unique(season, recipient)row, already tested) is what prevents re-paying a succeeded recipient on rerun; added a test exercising that guarantee throughreconcileSeasonDistributiondirectly.Closes #1854
#1855: Analytics retention computation for cohorts with no return visits
computeRetentionalready guardstotalUsers > 0before dividing, so it already returns0(notNaN) for a cohort with signups but no returns, and a defined empty result for zero signups, but none of this had test coverage. Added a dedicated spec covering: zero return visits reports 0%, zero signups returns a defined empty result rather than throwing, and full retention reports 100%.Closes #1855
Test plan
backend:npx jest(full suite) - all failures are pre-existing and unrelated to this diff (missing entity modules inaccount/achievementsspecs, etc.); every file touched in this PR passes in full.backend:npx tsc --noEmit- no new type errors introduced (baseline pre-existing errors unchanged, confirmed by diffing against a stash of these changes).backend:npx eslint "src/**/*.ts"- no new errors or warnings in touched files.backend:npx prettier --checkon all touched files.frontend:npx vitest run(full suite) - all failures are pre-existing and unrelated (wallet modal/hook tests);interactive-chart.test.tsxpasses in full (9/9).frontend:npx tsc --noEmit- no errors in touched files.