Merged
Conversation
- Add TeenEmailMapping model to map teen OAuth email to parent profiles - Add admin UI for configuring teen email mappings via Django admin - Modify JWT generation to return parent's tokens + teen's profile when teen logs in
Contributor
📝 WalkthroughWalkthroughA new Changes
Sequence DiagramsequenceDiagram
participant Client
participant JWT Endpoint
participant TeenEmailMapping DB
participant Token Generator
participant Client Response
Client->>JWT Endpoint: POST /get_jwt (oauth_email=user@teen.com)
JWT Endpoint->>TeenEmailMapping DB: lookup(oauth_email, select_related)
alt Mapping Found
TeenEmailMapping DB-->>JWT Endpoint: TeenEmailMapping object
JWT Endpoint->>Token Generator: get_delegated_tokens(parent_user, teen_profile)
Token Generator-->>JWT Endpoint: access, refresh tokens
JWT Endpoint->>Client Response: {access, refresh, active_profile_id, is_teen_delegated=true}
else No Mapping
TeenEmailMapping DB-->>JWT Endpoint: None
JWT Endpoint->>Token Generator: generate_tokens(authenticated_user)
Token Generator-->>JWT Endpoint: access, refresh tokens
JWT Endpoint->>Client Response: {access, refresh, is_teen_delegated=false}
end
Client Response-->>Client: JWT tokens
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
back/bots/admin.py (1)
68-74: Considerlist_select_relatedto avoid admin N+1 queries.
get_list_displayshowsteen_profileandparent_account; addinglist_select_relatedwill reduce query count on large mapping tables.♻️ Suggested refactor
class TeenEmailMappingAdmin(admin.ModelAdmin): + list_select_related = ('teen_profile', 'parent_account') + def get_readonly_fields(self, request, obj=None): return ['created_at', 'modified_at']🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@back/bots/admin.py` around lines 68 - 74, Add eager loading to the TeenEmailMappingAdmin to avoid admin N+1 queries: in the TeenEmailMappingAdmin class (where get_list_display is defined) add a list_select_related attribute or override get_queryset to call select_related for the related fields 'teen_profile' and 'parent_account' so the Django admin fetches those relations in one query instead of issuing per-row queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@back/bots/models/teen_email_mapping.py`:
- Around line 7-20: Add a UUID public identifier field to the TeenEmailMapping
model: define a field named teen_email_mapping_id as
models.UUIDField(default=uuid.uuid4, editable=False, unique=True, db_index=True)
on the TeenEmailMapping class, import uuid at the top of the file, and create a
migration so the new column is added; ensure the field follows the same naming
and uniqueness pattern used by other models (e.g., profile_id, bot_id) and is
placed alongside the existing model fields.
- Around line 13-24: The TeenEmailMapping model allows mismatched teen_profile
and parent_account; add model-level validation by implementing
TeenEmailMapping.clean() to raise django.core.exceptions.ValidationError when
self.teen_profile.user != self.parent_account.user (or equivalent user
relationship), and ensure this runs on save by calling full_clean() in
TeenEmailMapping.save() (or configure the admin ModelAdmin to call
full_clean()/validate on save_model) so the admin UI and programmatic saves
cannot persist inconsistent mappings; keep the existing unique_together intact.
In `@back/bots/views/get_jwt.py`:
- Around line 16-25: Update get_delegated_tokens to accept the full mapping
object (e.g., rename parameter to mapping) instead of teen_profile, validate
that teen_profile.user equals mapping.parent_account before issuing tokens, and
raise a custom exception (e.g., DelegationMappingError) if the mapped
parent_account and teen_profile.user do not match; then use
mapping.parent_account when creating the RefreshToken
(RefreshToken.for_user(mapping.parent_account)) and update the caller to pass
the mapping object into get_delegated_tokens. Ensure the new exception is
defined and used by the existing error handling path so validation failures are
caught alongside missing mappings.
- Around line 41-46: The TeenEmailMapping lookup uses .get() on a non-unique
field and can raise MultipleObjectsReturned and also performs a case-sensitive
match; update the lookup in the delegated login path (the block that currently
calls TeenEmailMapping.objects.select_related(...).get(oauth_email=user.email)
and then get_delegated_tokens(user, mapping.teen_profile)) to perform a
case-insensitive lookup using oauth_email__iexact, handle
MultipleObjectsReturned by deterministically selecting one mapping (for example
the first mapping ordered by id or parent_account) or return a controlled error,
and fall back to the existing RefreshToken.for_user(user) path when no mapping
is found; ensure you also catch django.core.exceptions.MultipleObjectsReturned
around the original .get() if you prefer to keep it and add explicit handling
there.
---
Nitpick comments:
In `@back/bots/admin.py`:
- Around line 68-74: Add eager loading to the TeenEmailMappingAdmin to avoid
admin N+1 queries: in the TeenEmailMappingAdmin class (where get_list_display is
defined) add a list_select_related attribute or override get_queryset to call
select_related for the related fields 'teen_profile' and 'parent_account' so the
Django admin fetches those relations in one query instead of issuing per-row
queries.
🪄 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: e85fb515-0bad-4422-805c-f088614337a9
📒 Files selected for processing (5)
back/bots/admin.pyback/bots/migrations/0034_teenemailmapping.pyback/bots/models/__init__.pyback/bots/models/teen_email_mapping.pyback/bots/views/get_jwt.py
Comment on lines
+7
to
+20
| class TeenEmailMapping(models.Model): | ||
| teen_profile = models.OneToOneField( | ||
| Profile, | ||
| on_delete=models.CASCADE, | ||
| related_name='teen_email_mapping' | ||
| ) | ||
| parent_account = models.ForeignKey( | ||
| UserAccount, | ||
| on_delete=models.CASCADE, | ||
| related_name='teen_mappings' | ||
| ) | ||
| oauth_email = models.EmailField(max_length=254) | ||
| created_at = models.DateTimeField(auto_now_add=True) | ||
| modified_at = models.DateTimeField(auto_now=True) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine its complete contents
cat -n back/bots/models/teen_email_mapping.py 2>/dev/null || echo "File not found or error reading"Repository: tpaulshippy/bots
Length of output: 1017
🏁 Script executed:
# List all model files in the bots/models directory to understand the structure
ls -la back/bots/models/*.py 2>/dev/null | head -20Repository: tpaulshippy/bots
Length of output: 857
🏁 Script executed:
# Search for UUID field usage patterns in existing models
rg "UUIDField" back/bots/models/ -A 2 -B 2Repository: tpaulshippy/bots
Length of output: 1996
🏁 Script executed:
# Check for uuid imports in model files
rg "import uuid" back/bots/models/Repository: tpaulshippy/bots
Length of output: 251
🏁 Script executed:
# Verify the exact import and field pattern in one of the existing models
head -15 back/bots/models/bot.pyRepository: tpaulshippy/bots
Length of output: 413
🏁 Script executed:
# Check if teen_email_mapping_id follows the naming convention used elsewhere
rg "_id = models.UUIDField" back/bots/models/ | head -10Repository: tpaulshippy/bots
Length of output: 598
Add a UUID public identifier field to this model.
TeenEmailMapping is missing the {model_name}_id UUID field required by the model guidelines. All other models in back/bots/models/ implement this pattern: profile_id, bot_id, chat_id, device_id, and message_id.
♻️ Suggested change
+import uuid
from django.conf import settings
from django.db import models
from .profile import Profile
from .user_account import UserAccount
class TeenEmailMapping(models.Model):
+ teen_email_mapping_id = models.UUIDField(default=uuid.uuid4, editable=False, unique=True)
teen_profile = models.OneToOneField(
Profile,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@back/bots/models/teen_email_mapping.py` around lines 7 - 20, Add a UUID
public identifier field to the TeenEmailMapping model: define a field named
teen_email_mapping_id as models.UUIDField(default=uuid.uuid4, editable=False,
unique=True, db_index=True) on the TeenEmailMapping class, import uuid at the
top of the file, and create a migration so the new column is added; ensure the
field follows the same naming and uniqueness pattern used by other models (e.g.,
profile_id, bot_id) and is placed alongside the existing model fields.
back/bots/views/get_jwt.py
Outdated
Comment on lines
+41
to
+46
| mapping = TeenEmailMapping.objects.select_related('teen_profile', 'parent_account').get( | ||
| oauth_email=user.email | ||
| ) | ||
| response_data = get_delegated_tokens(user, mapping.teen_profile) | ||
| except TeenEmailMapping.DoesNotExist: | ||
| refresh = RefreshToken.for_user(user) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the TeenEmailMapping model definition
find . -type f -name "*.py" | xargs grep -l "class TeenEmailMapping" | head -5Repository: tpaulshippy/bots
Length of output: 121
🏁 Script executed:
# Locate the get_jwt.py file
find . -type f -name "get_jwt.py" | head -5Repository: tpaulshippy/bots
Length of output: 88
🏁 Script executed:
# Search for unique constraints on TeenEmailMapping
rg "TeenEmailMapping" -A 20 -B 2 | head -100Repository: tpaulshippy/bots
Length of output: 5832
🏁 Script executed:
cat -n back/bots/views/get_jwt.pyRepository: tpaulshippy/bots
Length of output: 2266
🏁 Script executed:
cat -n back/bots/models/teen_email_mapping.pyRepository: tpaulshippy/bots
Length of output: 1017
🏁 Script executed:
# Check if there's parent_account context available from request/auth
rg "parent_account" back/bots/views/ -B 3 -A 3Repository: tpaulshippy/bots
Length of output: 497
🏁 Script executed:
# Check error handling patterns in the codebase
rg "MultipleObjectsReturned" back/bots/ -B 2 -A 2Repository: tpaulshippy/bots
Length of output: 42
Add exception handling for ambiguous email mappings in delegated login path.
Line 41–43 queries TeenEmailMapping.objects.get(oauth_email=user.email) on a non-unique field. The model permits multiple records per oauth_email across different parent_account values (via unique_together). When duplicates exist, .get() raises MultipleObjectsReturned, which is not caught, causing a 500 error during login.
Add explicit handling for this case and use case-insensitive email matching:
Suggested fix
try:
- mapping = TeenEmailMapping.objects.select_related('teen_profile', 'parent_account').get(
- oauth_email=user.email
- )
+ mapping = TeenEmailMapping.objects.select_related('teen_profile', 'parent_account').get(
+ oauth_email__iexact=user.email
+ )
response_data = get_delegated_tokens(user, mapping.teen_profile)
except TeenEmailMapping.DoesNotExist:
refresh = RefreshToken.for_user(user)
response_data = {
'access': str(refresh.access_token),
'refresh': str(refresh),
}
+ except TeenEmailMapping.MultipleObjectsReturned:
+ return JsonResponse(
+ {'detail': 'Multiple teen delegated mappings found for this OAuth email.'},
+ status=409
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| mapping = TeenEmailMapping.objects.select_related('teen_profile', 'parent_account').get( | |
| oauth_email=user.email | |
| ) | |
| response_data = get_delegated_tokens(user, mapping.teen_profile) | |
| except TeenEmailMapping.DoesNotExist: | |
| refresh = RefreshToken.for_user(user) | |
| mapping = TeenEmailMapping.objects.select_related('teen_profile', 'parent_account').get( | |
| oauth_email__iexact=user.email | |
| ) | |
| response_data = get_delegated_tokens(user, mapping.teen_profile) | |
| except TeenEmailMapping.DoesNotExist: | |
| refresh = RefreshToken.for_user(user) | |
| except TeenEmailMapping.MultipleObjectsReturned: | |
| return JsonResponse( | |
| {'detail': 'Multiple teen delegated mappings found for this OAuth email.'}, | |
| status=409 | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@back/bots/views/get_jwt.py` around lines 41 - 46, The TeenEmailMapping lookup
uses .get() on a non-unique field and can raise MultipleObjectsReturned and also
performs a case-sensitive match; update the lookup in the delegated login path
(the block that currently calls
TeenEmailMapping.objects.select_related(...).get(oauth_email=user.email) and
then get_delegated_tokens(user, mapping.teen_profile)) to perform a
case-insensitive lookup using oauth_email__iexact, handle
MultipleObjectsReturned by deterministically selecting one mapping (for example
the first mapping ordered by id or parent_account) or return a controlled error,
and fall back to the existing RefreshToken.for_user(user) path when no mapping
is found; ensure you also catch django.core.exceptions.MultipleObjectsReturned
around the original .get() if you prefer to keep it and add explicit handling
there.
- Add oauth_email field to Profile model - Simplify get_jwt to lookup Profile by oauth_email - Remove TeenEmailMapping model and admin - Update Profile admin to show oauth_email in list view
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
1 participant
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
oauth_emailfield to Profile model for delegated teen login/admin/bots/profile/- edit a profile to add the teen's OAuth emailImplementation Details
oauth_emailfield to Profile model (nullable, blankable)get_jwtview to check ifuser.emailmatches any Profile'soauth_email; if so, returns parent's JWT tokens withactive_profile_idset to that profileUsage
oauth_emailto the teen's Google/Apple emailTesting
oauth_emailto a test email address