Fix dark mode text visibility and card creation race condition#14
Conversation
- Add theme-aware text color to TextInput components in cardEdit.tsx - Fix IntegrityError by locking flashcard rows during card creation to prevent race conditions on order field
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 45 minutes and 33 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds database-level row locking to prevent race conditions during flashcard creation and applies theme-aware text colors to the CardEdit input fields. The backend change serializes order assignments; the frontend change ensures input text inherits the current theme's color scheme. ChangesBackend Order Race Prevention
Frontend Theme Color Application
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
front/app/flashcards/cardEdit.tsx (1)
107-109: ⚡ Quick winMake placeholder text theme-aware too.
The input text now adapts to theme, but placeholder text is still fixed to
#888, which can be inconsistent/low-contrast across themes. Use a theme-derived placeholder color (e.g., viauseThemeColor) for both fields.💡 Proposed change
- const textColor = useThemeColor({}, 'text'); + const textColor = useThemeColor({}, "text"); + const placeholderColor = useThemeColor({}, "icon"); ... <TextInput style={[styles.input, styles.cardInput, { color: textColor }]} placeholder="Enter the question or term" - placeholderTextColor="`#888`" + placeholderTextColor={placeholderColor} value={cardFront} onChangeText={setCardFront} multiline /> ... <TextInput style={[styles.input, styles.cardInput, { color: textColor }]} placeholder="Enter the answer or definition" - placeholderTextColor="`#888`" + placeholderTextColor={placeholderColor} value={cardBack} onChangeText={setCardBack} multiline />Also applies to: 117-119
🤖 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 `@front/app/flashcards/cardEdit.tsx` around lines 107 - 109, The placeholderTextColor is hardcoded to "`#888`"; update the CardEdit component to derive a theme-aware placeholder color via useThemeColor (e.g., const placeholderColor = useThemeColor({ light: '`#888`', dark: '`#BBB`' }, 'placeholder') or similar) and pass placeholderColor to the placeholderTextColor prop on both TextInput fields (the ones using styles.input / styles.cardInput and textColor). Ensure you import/use useThemeColor where the inputs are defined and replace the literal "`#888`" at both input locations with the derived placeholderColor.
🤖 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 `@back/bots/viewsets/flashcard_viewset.py`:
- Line 5: The current create/update path in FlashcardViewSet that calls
serializer.save(...) can still raise IntegrityError for the
unique_flashcard_order_per_deck constraint; catch django.db.utils.IntegrityError
around the serializer.save call (in the methods handling create/update), log the
deck identifier and constraint name using the view's context (e.g.,
self.request.data.get('deck') or serializer.validated_data['deck']), and raise a
typed API error (e.g., return Response with 400 Bad Request or raise a DRF
ValidationError with a message like "Flashcard order must be unique per deck:
deck=<deck_id>") so the client receives a clear, descriptive error instead of a
500. Ensure the log includes the deck id and the constraint name
unique_flashcard_order_per_deck and that the exception handling covers both
create and update paths where serializer.save is invoked.
---
Nitpick comments:
In `@front/app/flashcards/cardEdit.tsx`:
- Around line 107-109: The placeholderTextColor is hardcoded to "`#888`"; update
the CardEdit component to derive a theme-aware placeholder color via
useThemeColor (e.g., const placeholderColor = useThemeColor({ light: '`#888`',
dark: '`#BBB`' }, 'placeholder') or similar) and pass placeholderColor to the
placeholderTextColor prop on both TextInput fields (the ones using styles.input
/ styles.cardInput and textColor). Ensure you import/use useThemeColor where the
inputs are defined and replace the literal "`#888`" at both input locations with
the derived placeholderColor.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59406aaa-8c0e-4604-b8a4-b01b802075fa
📒 Files selected for processing (2)
back/bots/viewsets/flashcard_viewset.pyfront/app/flashcards/cardEdit.tsx
There was a problem hiding this comment.
Pull request overview
This PR addresses two user-facing regressions: unreadable flashcard editor input text in dark mode (frontend) and occasional UNIQUE constraint failed errors when creating flashcards concurrently due to non-atomic MAX(order)+1 assignment (backend).
Changes:
- Frontend: Applies theme-aware text color to flashcard edit form
TextInputfields. - Backend: Attempts to prevent duplicate
ordervalues during concurrent flashcard creation by adding a locking query inside the create transaction.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| front/app/flashcards/cardEdit.tsx | Adds theme-derived text color to flashcard edit inputs to improve dark mode visibility. |
| back/bots/viewsets/flashcard_viewset.py | Adds a select_for_update() query intended to reduce flashcard order race conditions during create. |
Comments suppressed due to low confidence (2)
back/bots/viewsets/flashcard_viewset.py:79
select_for_update()is ignored by Django’s SQLite backend (seeback/server/settings.pyusesdjango.db.backends.sqlite3), so this added query won’t actually lock anything in the default environment. That means themax_ordercalculation can still race and hit theunique_flashcard_order_per_deckconstraint under concurrent requests. Consider a DB-agnostic fix such as retrying themax_order+save()inside the transaction onIntegrityError(recompute max and retry a few times), or switching to an atomic counter stored onDeck/separate sequence table instead ofMAX(order) + 1.
# Lock flashcard rows to prevent race conditions on order
Flashcard.objects.filter(deck=deck).select_for_update().order_by('-order').first()
max_order = Flashcard.objects.filter(deck=deck).aggregate(Max('order'))['order__max'] or -1
serializer.save(deck=deck, order=max_order + 1)
front/app/flashcards/cardEdit.tsx:111
- These
TextInputinstances are still hard-codingplaceholderTextColor="#888". If the goal is full dark/light theme correctness, consider sourcing placeholder color from the theme as well (or using the existingThemedTextInput, which centralizes this behavior).
<TextInput
style={[styles.input, styles.cardInput, { color: textColor }]}
placeholder="Enter the question or term"
placeholderTextColor="#888"
value={cardFront}
onChangeText={setCardFront}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from rest_framework.exceptions import NotFound | ||
| from django.db.models import Count, Max | ||
| from django.db import transaction | ||
| from django.db.utils import IntegrityError |
| const [cardFront, setCardFront] = useState(front || ""); | ||
| const [cardBack, setCardBack] = useState(back || ""); | ||
| const textColor = useThemeColor({}, 'text'); | ||
|
|
||
| useEffect(() => { |
- Removed select_for_update() calls since SQLite doesn't support row locking - Implemented retry logic (max 3 attempts) for concurrent card creation - On IntegrityError, recompute max_order and retry instead of failing - This is a DB-agnostic fix that works with both SQLite and PostgreSQL
- Replace TextInput with ThemedTextInput component - Remove manual useThemeColor hook and textColor variable - ThemedTextInput handles text and placeholder colors consistently - Keeps theming logic DRY across the app
Summary
Fixes two issues:
1. Dark mode text visibility (Frontend)
Text in the card edit form's TextInput fields was not visible in dark mode. The text color was defaulting to system colors, resulting in dark text on a dark background.
Fix: Added theme-aware text color using the
useThemeColorhook to ensure text is visible in both light and dark modes.2. Card creation IntegrityError (Backend)
When creating new flashcards concurrently, a race condition could cause
IntegrityError: UNIQUE constraint failed: bots_flashcard.deck_id, bots_flashcard.orderbecause multiple requests could calculate the samemax_ordervalue before any of them saved.Fix: Added
select_for_update()on the flashcard rows within the transaction to lock them during the order calculation, preventing concurrent requests from reading stale data.Testing
Summary by CodeRabbit
Bug Fixes
Style