Skip to content

Conversation

@smurflo2
Copy link
Collaborator

@smurflo2 smurflo2 commented Dec 18, 2025

Overview

🎟 Relevant Jira Issues

LC-1229

📚 What is the context and goal of this PR?

During the login with email flow, a user reported that the flow reset back to the "enter email" step (rather than staying on the "enter code" step) when they switched to their email and back.

This PR adds more robust logic for when the "enter code" step should be shown, so it now persists even when the page/app is refreshed.

🥴 TL; RL:

Stay on this step even if page is refreshed
image

💡 Feature Breakdown (screenshots & videos encouraged!)

🛠 Important tradeoffs made:

🔍 Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Chore (refactor, documentation update, etc)

💳 Does This Create Any New Technical Debt? ( If yes, please describe and add JIRA TODOs )

  • No
  • Yes

Testing

🔬 How Can Someone QA This?

  • Login with email
  • Refresh the "enter code" page (/ if you're testing on native: switch apps to email and then switch back)
  • Confirm the page stays on that page after reloading
  • Enter the code (note: it took ~4 minutes for the code to be sent to me while running locally 🤷‍♂️)
  • You should be logged in successfully

📱 🖥 Which devices would you like help testing on?

🧪 Code Coverage

Documentation

📝 Documentation Checklist

User-Facing Docs (docs/docs.learncard.com)

  • Tutorial — New capability that users need to learn (docs/tutorials/)
  • How-To Guide — New workflow or integration (docs/how-to-guides/)
  • Reference — New/changed API, config, or SDK method (docs/sdks/)
  • Concept — New mental model or architecture explanation (docs/core-concepts/)
  • App Flows — Changes to LearnCard App or ScoutPass user flows (docs/apps/)

Internal/AI Docs

  • CLAUDE.md — New pattern, flow, or context that AI assistants need
  • Code comments/JSDoc — Complex logic that needs inline explanation

Visual Documentation

  • Mermaid diagram — Complex flow, state machine, or architecture
B[LearnCard] --> C[Network]
-->

#### 💭 Documentation Notes
<!-- If no docs needed, briefly explain why (e.g., "Internal refactor, no API changes") -->
Simple bug fix

# ✅ PR Checklist
- [x] Related to a Jira issue ([create one if not](https://welibrary.atlassian.net/jira/software/projects/WE/boards/2))
- [x] My code follows **style guidelines** (eslint / prettier)
- [x] I have **manually tested** common end-2-end cases
- [x] I have **reviewed** my code
- [x] I have **commented** my code, particularly where ambiguous
- [ ] New and existing **unit tests pass** locally with my changes
- [ ] I have completed the **Documentation Checklist** above (or explained why N/A)

### 🚀 Ready to squash-and-merge?:
- [x] Code is backwards compatible
- [x] There is **not** a "Do Not Merge" label on this PR
- [x] I have thoughtfully considered the security implications of this change.
- [x] This change does not expose new public facing endpoints that do not have authentication


[LC-1229]: https://welibrary.atlassian.net/browse/LC-1229?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
<!--start_gitstream_placeholder-->
### ✨ PR Description
Purpose: Fix email verification step persistence during page refresh by checking both URL parameter and stored email state to prevent users from losing progress.

Main changes:
- Updated shouldVerifyCode logic to check both verifyCode URL query parameter and verificationEmail from storage
- Applied Boolean coercion to ensure consistent truthy/falsy evaluation across refresh scenarios
- Replicated fix across learn-card-app and scouts applications for consistent behavior

_Generated by LinearB AI and added by gitStream._
<sub>AI-generated content may contain inaccuracies. Please verify before using.
💡 **Tip:** You can customize your AI Description using **Guidelines** [Learn how](https://docs.gitstream.cm/automation-actions/#describe-changes)</sub>
<!--end_gitstream_placeholder-->

@changeset-bot
Copy link

changeset-bot bot commented Dec 18, 2025

⚠️ No Changeset found

Latest commit: c88e233

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented Dec 18, 2025

Deploy Preview for staging-learncardapp canceled.

Name Link
🔨 Latest commit c88e233
🔍 Latest deploy log https://app.netlify.com/projects/staging-learncardapp/deploys/69457c8165768e0009440289

@netlify
Copy link

netlify bot commented Dec 18, 2025

Deploy Preview for learncarddocs canceled.

Name Link
🔨 Latest commit c88e233
🔍 Latest deploy log https://app.netlify.com/projects/learncarddocs/deploys/69457c81caa94200088b321d

@github-actions
Copy link
Contributor

👋 Hey there! It looks like you modified code, but didn't update the documentation in /docs.

If this PR introduces new features, changes APIs, or modifies behavior that users or developers need to know about, please consider updating the docs.


🏄 Windsurf Tip

You can ask Windsurf to help:

"Analyze the changes in this PR and update the gitbook docs in /docs accordingly."

Windsurf will review your changes and suggest appropriate documentation updates based on what was modified.


📚 Documentation Guide
Change Type Doc Location
New feature/API docs/tutorials/ or docs/how-to-guides/
SDK/API changes docs/sdks/
New concepts docs/core-concepts/
App UI/UX flows docs/apps/ (LearnCard App, ScoutPass)
Internal patterns CLAUDE.md

This is an automated reminder. If no docs are needed, feel free to ignore this message.

Copy link
Contributor

@gitstream-cm gitstream-cm bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ PR Review

LGTM

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

@gitstream-cm
Copy link
Contributor

gitstream-cm bot commented Dec 18, 2025

🥷 Code experts: TaylorBeeston

TaylorBeeston has most 👩‍💻 activity in the files.
TaylorBeeston has most 🧠 knowledge in the files.

See details

apps/learn-card-app/src/pages/login/forms/EmailForm.tsx

Activity based on git-commit:

TaylorBeeston
DEC
NOV 462 additions & 0 deletions
OCT
SEP
AUG
JUL

Knowledge based on git-blame:
TaylorBeeston: 100%

apps/scouts/src/pages/login/forms/EmailForm.tsx

Activity based on git-commit:

TaylorBeeston
DEC
NOV 404 additions & 0 deletions
OCT
SEP
AUG
JUL

Knowledge based on git-blame:
TaylorBeeston: 100%

✨ Comment /gs review for LinearB AI review. Learn how to automate it here.

@Custard7
Copy link
Collaborator

@claude please review this PR and provide a recommendation on merging

@claude
Copy link
Contributor

claude bot commented Dec 19, 2025

Claude finished @Custard7's task —— View job


PR Review and Analysis

Todo List

  • Read and analyze the changed files
  • Understand the current implementation and the fix
  • Check for potential issues or improvements
  • Provide merge recommendation

Code Review

Change Summary

The PR fixes a user experience issue in the email login verification flow by making the "verify code" step persist across page refreshes. The change is minimal but effective:

Before:

const shouldVerifyCode = query.get('verifyCode');

After:

const shouldVerifyCode = Boolean(query.get('verifyCode') || verificationEmail);

Analysis

Strengths

  1. Targeted Fix: The change directly addresses the reported issue where users lose their place in the verification flow when refreshing/switching apps
  2. Consistent Implementation: Both apps (learn-card-app and scouts) receive identical fixes, maintaining consistency
  3. Logical Approach: Uses stored email from redirectStore as a fallback indicator when URL parameter is lost
  4. Minimal Risk: Very small change surface area reduces chance of introducing bugs
  5. Boolean Coercion: Proper use of Boolean() ensures consistent truthy evaluation

Code Quality

  • Clean, readable code
  • Follows existing patterns in the codebase
  • Proper TypeScript usage
  • No security concerns identified

Architecture Alignment

  • Leverages existing redirectStore.get.email() infrastructure
  • Maintains existing flow logic in useEffect that depends on shouldVerifyCode
  • No breaking changes to component API

Verification

The fix logic is sound:

  1. If verifyCode query param exists → show verification step ✅
  2. If stored verificationEmail exists → show verification step ✅
  3. Both conditions use OR logic, so either triggers the verification flow ✅
  4. Boolean() ensures consistent truthy evaluation ✅

Looking at lines apps/learn-card-app/src/pages/login/forms/EmailForm.tsx:88-93, the useEffect properly responds to shouldVerifyCode changes and sets the correct step.

Recommendation: ✅ APPROVE FOR MERGE

This is a well-executed bug fix that:

  • Solves the reported user experience issue
  • Uses minimal, safe code changes
  • Maintains consistency across both applications
  • Follows established patterns and doesn't introduce technical debt
  • Has been tested according to the PR description

The only note is that a changeset may be needed if this should trigger version bumps for the affected applications, but that's a process matter rather than a code quality issue.

Copy link
Collaborator

@goblincore goblincore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smurflo2 LGTM!

@goblincore goblincore merged commit 8cf4423 into main Dec 19, 2025
24 checks passed
@goblincore goblincore deleted the lc-1229 branch December 19, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants