feat(waves): onboarding checklist card for new users#3333
Conversation
Greptile SummaryThis PR adds a Waves onboarding checklist for new users. The main changes are:
Confidence Score: 4/5This is close, but the persistence ordering issue should be fixed before merging.
src/screens/waves/children/wavesOnboardingChecklist.tsx Important Files Changed
Reviews (5): Last reviewed commit: "fix(waves): centralize onboarding flag w..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d02d9d23b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const streak = quests.streak?.current ?? 0; | ||
| const items = [ | ||
| item('wave', dailyProgress(quests, 'post') > 0), |
There was a problem hiding this comment.
On mobile _submitWave posts through _submitReply with PostTypes.WAVE and a non-empty waves-container parent (src/components/quickPostModal/usePostSubmitter.ts:442-445), and the SDK records useComment broadcasts with a parent as comment activity. The quests endpoint therefore advances the comment quest for a Wave, not post, so using this card's “Post your first Wave” CTA leaves this item unchecked while potentially checking “Reply to someone”; the checklist cannot reach all-done from the intended flow unless the user also publishes a separate long-form post.
Useful? React with 👍 / 👎.
| useEffect(() => { | ||
| if (state?.allComplete && persisted && !persisted.celebrated) { | ||
| setCelebrating(true); | ||
| _persist({ ...persisted, celebrated: true }); |
There was a problem hiding this comment.
When the last unchecked item completes from live quest data, both effects run in the same commit: the latch effect writes { ...persisted, done }, then this celebration write uses the old persisted object and can overwrite storage/state without the newly latched done ids. After the daily quest reset or a remount, celebrated is true but the missing ids are no longer completed, causing the supposedly hidden checklist to reappear with items unchecked.
Useful? React with 👍 / 👎.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Waves onboarding state derivation, a persisted checklist card, screen integration in Waves and Feed, wave latch emission, and localized onboarding strings. ChangesWaves onboarding checklist feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
🤖 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 `@src/screens/waves/children/wavesOnboardingChecklist.tsx`:
- Around line 75-101: The onboarding persistence in wavesOnboardingChecklist.tsx
has a race between the latch and celebration useEffect blocks, both spreading
the same persisted snapshot and overwriting each other. Merge the logic in the
wavesOnboardingChecklist component into a single effect (or otherwise ensure one
atomic update) so the newly completed item is latched and celebrated together
from one consistent persisted value before calling _persist. Keep the update
keyed off state, persisted, and state?.allComplete, and preserve the invariant
that completed items never get removed from persisted.done.
In `@src/utils/wavesOnboarding.ts`:
- Around line 26-36: The `isNewAccount` helper assumes `account.created` is
always a string, but `selectCurrentAccount` may provide a truthy partial object
and `created.indexOf(...)` can throw during Waves onboarding rendering. Update
`isNewAccount` to guard `created` with a `typeof created === 'string'` check
before calling `Date.parse`, and when it is missing or invalid, fall back to the
`post_count` threshold logic in the same function.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1d23b53-afd6-469b-afff-d7158f9862be
📒 Files selected for processing (6)
src/config/locales/en-US.jsonsrc/screens/waves/children/wavesOnboardingChecklist.tsxsrc/screens/waves/screen/wavesScreen.tsxsrc/screens/waves/styles/wavesOnboardingChecklist.styles.tssrc/utils/wavesOnboarding.test.tssrc/utils/wavesOnboarding.ts
| export const isNewAccount = ( | ||
| account: { created: string; post_count: number }, | ||
| now = Date.now(), | ||
| ): boolean => { | ||
| // Hive timestamps come without a timezone marker but are UTC. | ||
| const { created } = account; | ||
| const createdAt = Date.parse(created.indexOf('Z') === -1 ? `${created}Z` : created); | ||
| const withinWindow = | ||
| Number.isFinite(createdAt) && now - createdAt <= NEW_ACCOUNT_WINDOW_DAYS * DAY_MS; | ||
| return withinWindow || account.post_count < NEW_ACCOUNT_MAX_POSTS; | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect selectCurrentAccount and the Account type to confirm whether
# `created`/`post_count` can be missing on a truthy currentAccount object.
fd -e ts -e tsx selectors src/redux --type f | xargs rg -nP -A 10 'selectCurrentAccount'
rg -nP -A 5 'interface Account\b|type Account\b' srcRepository: ecency/ecency-mobile
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first.
fd -e ts -e tsx 'wavesOnboarding.ts|wavesOnboardingChecklist.tsx|selectors/index.ts|account' src | sort
printf '\n## wavesOnboarding.ts\n'
cat -n src/utils/wavesOnboarding.ts | sed -n '1,120p'
printf '\n## wavesOnboardingChecklist.tsx\n'
cat -n src/screens/waves/children/wavesOnboardingChecklist.tsx | sed -n '1,180p'
printf '\n## redux/selectors/index.ts (current account area)\n'
cat -n src/redux/selectors/index.ts | sed -n '160,210p'
printf '\n## account-related type definitions / state shape\n'
rg -n --glob '*.ts' --glob '*.tsx' -A 8 -B 4 'currentAccount|interface Account|type Account|post_count|created:' src/redux src/screens src/utils | sed -n '1,240p'Repository: ecency/ecency-mobile
Length of output: 33570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow the search to the specific flow and account shape.
printf '## wavesOnboarding.ts\n'
cat -n src/utils/wavesOnboarding.ts | sed -n '1,140p'
printf '\n## wavesOnboardingChecklist.tsx\n'
cat -n src/screens/waves/children/wavesOnboardingChecklist.tsx | sed -n '1,220p'
printf '\n## selectCurrentAccount and related account selectors\n'
cat -n src/redux/selectors/index.ts | sed -n '170,205p'
printf '\n## Account type / current account state\n'
rg -n -A 6 -B 4 'created:|post_count:|currentAccount' src/redux src | sed -n '1,260p'Repository: ecency/ecency-mobile
Length of output: 30674
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on the account model and any places that construct/update currentAccount.
rg -n -A 8 -B 4 '\bcurrentAccount\b|post_count|created:' src/redux src/screens src/utils src/providers | sed -n '1,260p'
printf '\n## files that look like account models\n'
fd -i 'account' src/redux src | sortRepository: ecency/ecency-mobile
Length of output: 18266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the account slice/reducer to see what shape `currentAccount` can have.
fd -e ts -e tsx 'account' src/redux src | sort
printf '\n## account reducer / slice candidates\n'
rg -n --glob '*.ts' --glob '*.tsx' -A 12 -B 6 'currentAccount\s*:|updateCurrentAccount|setCurrentAccount|selectCurrentAccount' src/redux src | sed -n '1,260p'Repository: ecency/ecency-mobile
Length of output: 17548
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the onboarding helper signature and its callers.
rg -n -A 12 -B 6 'deriveWavesOnboardingState|isNewAccount\(' src/utils src/screens | sed -n '1,220p'Repository: ecency/ecency-mobile
Length of output: 15805
Guard created before parsing it
selectCurrentAccount can be a truthy partial object, so created.indexOf(...) can throw while Waves onboarding is rendering. Check typeof created === 'string' first and fall back to the post-count rule.
🤖 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 `@src/utils/wavesOnboarding.ts` around lines 26 - 36, The `isNewAccount` helper
assumes `account.created` is always a string, but `selectCurrentAccount` may
provide a truthy partial object and `created.indexOf(...)` can throw during
Waves onboarding rendering. Update `isNewAccount` to guard `created` with a
`typeof created === 'string'` check before calling `Date.parse`, and when it is
missing or invalid, fall back to the `post_count` threshold logic in the same
function.
| ) => { | ||
| const next = updater(memoryState.get(username) ?? {}); | ||
| memoryState.set(username, next); | ||
| setItemToStorage(_storageKey(username), next); |
There was a problem hiding this comment.
_updatePersisted now merges against the module-level state, which keeps mounted checklist instances in sync during the current session. The persisted write is still an unordered whole-object setItemToStorage call, though. If a latch or celebration write and a dismiss write happen close together, the older storage write can resolve last and overwrite the newer object on disk. After an app restart, the module map is empty, so the stale stored value can drop dismissed, done, or celebrated and make a dismissed or completed checklist appear again. The per-user storage writes need to be serialized or coalesced so older writes cannot land after newer state.
1 participant
Mobile counterpart of ecency/vision-next#1116: a dismissible "Getting started" checklist card at the top of the Waves screen for new accounts (younger than 30 days or fewer than 10 posts), nudging the first wave as the first action.
Full jest suite 595 passed / 0 failures; prettier and eslint clean on touched files.
Summary by CodeRabbit