feat(users): implement bulk user creation from template with username strategies #108
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughBulk user creation feature from templates is added, enabling creation of multiple users with configurable username strategies (random or sequence). The feature spans database operations, models, API endpoints, and Telegram handlers with validation and subscription URL generation. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Admin
participant API as API Endpoint
participant Op as UserOperation
participant CRUD as CRUD Layer
participant DB as Database
User->>API: POST /s/bulk/from_template<br/>(count, strategy, username, ...)
API->>Op: bulk_create_users_from_template()
activate Op
Op->>Op: Validate template & strategy
alt Strategy == Random
Op->>Op: _generate_usernames(strategy=random)
Note over Op: Generate random usernames<br/>(count times)
else Strategy == Sequence
Op->>Op: _generate_usernames(strategy=sequence,<br/>base_username, start_number)
Note over Op: Generate sequential usernames<br/>(base_username + suffix)
end
Op->>Op: _build_bulk_user_models(usernames)
Note over Op: Construct UserCreate models<br/>from candidate usernames
Op->>Op: _filter_existing_usernames(new_users)
Op->>CRUD: Check DB for existing usernames
CRUD->>DB: Query users table
DB-->>CRUD: Existing usernames
CRUD-->>Op: UserCreate list (duplicates removed)
Op->>Op: _persist_bulk_users(filtered_users)
Op->>CRUD: create_users_bulk(users, groups, admin)
CRUD->>DB: Batch insert users + associations
DB-->>CRUD: Created user records
CRUD-->>Op: Created User list + attributes
Op->>Op: Generate subscription URLs
Op->>Op: Emit notifications
deactivate Op
Op-->>API: BulkUsersCreateResponse<br/>(created_count, subscription_urls)
API-->>User: 201 with URLs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/telegram/keyboards/bulk_actions.py (1)
60-74: Consider usingadjust(1, repeat=True)for consistency.The
adjust(1, 1)call only sets layout for the first two buttons (random and sequence), but there are three buttons including the back button. While this may work due to default behavior, usingadjust(1, repeat=True)would be more explicit and consistent withBulkTemplateSelector(line 57).- self.adjust(1, 1) + self.adjust(1, repeat=True)app/operation/user.py (1)
129-141: Dead code:widthis always 0, makingzfillineffective.The
widthvariable is initialized to0and never updated, so the conditionif width:(line 137) is always false, and thezfill(width)call never executes. If zero-padding is intended for sequence usernames,widthneeds to be calculated or made configurable. Otherwise, these lines can be simplified.prefix = base_username - width = 0 - inferred_start_number = 1 generated: list[str] = [] - current = start_number if start_number is not None else inferred_start_number + current = start_number if start_number is not None else 1 for _ in range(count): - suffix = str(current) - if width: - suffix = suffix.zfill(width) - generated.append(f"{prefix}{suffix}") + generated.append(f"{prefix}{current}") current += 1 return generatedapp/telegram/handlers/admin/bulk_actions.py (1)
108-130: Minor inconsistency: stored strategy value is unused.Line 127 stores
strategy.valuein state, butbulk_template_start_numberhardcodesUsernameGenerationStrategy.sequenceon line 197 rather than retrieving it. This works correctly since only the sequence path reaches that handler, but consider removing the unused state update or using the stored value for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/db/crud/user.py(3 hunks)app/models/user.py(2 hunks)app/operation/user.py(8 hunks)app/routers/user.py(2 hunks)app/telegram/handlers/admin/bulk_actions.py(1 hunks)app/telegram/keyboards/bulk_actions.py(3 hunks)app/telegram/utils/forms.py(1 hunks)app/telegram/utils/texts.py(5 hunks)tests/api/test_user.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
app/telegram/keyboards/bulk_actions.py (2)
app/models/user.py (1)
UsernameGenerationStrategy(195-197)app/telegram/keyboards/admin.py (3)
AdminPanel(17-43)AdminPanelAction(9-14)Callback(18-19)
tests/api/test_user.py (5)
tests/api/conftest.py (1)
access_token(440-445)tests/api/test_bulk.py (1)
setup_groups(16-19)tests/api/helpers.py (4)
create_user_template(154-176)unique_name(12-13)delete_user(149-151)delete_user_template(179-181)app/routers/user_template.py (1)
create_user_template(19-32)app/db/models.py (2)
group_ids(210-211)group_ids(346-347)
app/telegram/utils/texts.py (1)
app/telegram/handlers/admin/bulk_actions.py (1)
bulk_create_from_template(66-73)
app/db/crud/user.py (1)
app/db/models.py (6)
DataLimitResetStrategy(113-118)User(121-282)expire(164-167)expire(170-171)expire(174-175)NextPlan(303-313)
app/telegram/handlers/admin/bulk_actions.py (8)
app/models/validators.py (1)
validate_username(127-141)app/operation/__init__.py (1)
OperatorType(27-33)app/operation/user.py (1)
bulk_create_users_from_template(617-661)app/operation/user_template.py (2)
UserTemplateOperation(21-68)get_user_templates(65-68)app/telegram/keyboards/base.py (2)
CancelKeyboard(12-19)Callback(18-19)app/telegram/keyboards/bulk_actions.py (7)
BulkActionPanel(19-40)BulkAction(12-16)BulkTemplateSelector(43-57)UsernameStrategySelector(60-74)Callback(20-22)Callback(44-45)Callback(61-62)app/telegram/utils/forms.py (1)
BulkCreateFromTemplate(26-30)app/models/user_template.py (1)
validate_username(45-46)
app/models/user.py (2)
app/telegram/utils/forms.py (1)
CreateUserFromTemplate(13-14)dashboard/src/service/api/index.ts (1)
CreateUserFromTemplate(1641-1645)
app/routers/user.py (5)
app/models/user.py (2)
BulkUsersCreateResponse(225-227)BulkUsersFromTemplate(205-222)app/operation/user.py (1)
bulk_create_users_from_template(617-661)app/db/base.py (1)
get_db(57-59)app/models/admin.py (1)
AdminDetails(48-64)app/routers/authentication.py (1)
get_current(37-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-databases (timescaledb)
🔇 Additional comments (22)
app/db/crud/user.py (2)
80-89: LGTM!The
get_existing_usernamesfunction is well-implemented with proper early exit for empty input and efficient IN query for batch lookup.
506-544: LGTM!The bulk creation function properly handles:
- Early exit for empty input
- Correct group assignment using
list(groups)to avoid shared reference issues- Proper sequencing with flush before NextPlan creation to ensure user IDs are available
- Single commit at the end for atomicity
app/telegram/utils/texts.py (1)
34-34: LGTM!The new button labels, prompts, and messages are clear, consistent with existing patterns, and properly support the bulk creation workflow.
Also applies to: 48-49, 64-68, 79-80, 270-277
app/telegram/utils/forms.py (1)
26-31: LGTM!The new
BulkCreateFromTemplatestate group properly captures all required fields for the Telegram-based bulk creation workflow.app/routers/user.py (1)
320-341: LGTM!The new bulk creation endpoint is well-structured with:
- Appropriate HTTP 201 status for resource creation
- Comprehensive error responses (400, 403, 404, 409)
- Clear documentation of the strategy options and parameters
- Consistent authentication pattern with existing template-based creation
tests/api/test_user.py (3)
356-394: LGTM!The sequence strategy test comprehensively validates:
- Correct username generation with start_number offset
- Proper template attribute inheritance (data_limit, status)
- Subscription URL generation
- Proper cleanup in finally block
397-434: LGTM!The random strategy test properly validates the bulk creation flow and uses group filtering to retrieve created users for verification and cleanup.
437-457: LGTM!Good negative test case validating that the API correctly rejects invalid combinations (random strategy with username) with HTTP 422.
app/models/user.py (1)
195-227: LGTM!The new models are well-designed:
UsernameGenerationStrategyenum cleanly defines the two strategiesBulkCreationBaseenforces count bounds (1-500) with sensible defaultBulkUsersFromTemplateproperly overridesusernameto be optional and includes comprehensive validation viamodel_validator- The validator correctly enforces mutual exclusivity rules between strategy and username/start_number fields
app/operation/user.py (6)
74-78: LGTM!Clean utility method for formatting Pydantic validation errors into readable strings.
145-164: LGTM!The method correctly handles bulk model building by:
- Deduplicating candidate usernames locally
- Silently skipping invalid usernames (appropriate for bulk operations to maximize throughput)
- Returning only successfully built models
166-174: LGTM!Efficient filtering with proper early exit and set-based lookup for O(1) existence checks.
176-196: LGTM!The persistence method properly:
- Handles empty input with early exit
- Uses bulk insert for efficiency
- Generates subscription URLs after successful creation
- Fires off notifications asynchronously
- Logs each creation for audit purposes
558-575: LGTM!Clean extraction of user creation logic from template. The method properly:
- Applies template's username prefix/suffix
- Formats validation errors for clear API responses
- Applies template settings after model construction
617-661: LGTM!The bulk creation orchestration is well-structured:
- Validates template before proceeding
- Generates candidate usernames based on strategy
- Filters existing usernames to prevent conflicts
- Groups are validated once (all users share template groups)
- Returns comprehensive response with subscription URLs and count
The duplicate validation of strategy/username (also in model) provides defense-in-depth.
app/telegram/handlers/admin/bulk_actions.py (7)
10-29: LGTM!The imports and global initialization follow the existing patterns in the file (matching
user_operations). The new dependencies align with the bulk creation feature requirements.
34-36: LGTM!Clean helper for polymorphic event handling.
38-58: LGTM!The chunking logic correctly accounts for newline separators and respects the Telegram message limit with adequate headroom.
65-73: LGTM!The handler correctly initiates the bulk creation flow. The template fetch returns all templates; disabled templates are filtered at creation time in the operation layer.
76-105: LGTM!The flow correctly captures the template selection and validates the count within acceptable bounds (1-500).
160-179: LGTM!The start number validation correctly handles optional input (empty string → None) and validates non-negative integers.
138-144: The original review comment is incorrect.UserValidator.validate_usernameis properly implemented as a static method inapp/models/validators.py(line 127) and can be called exactly as shown in line 140 ofbulk_actions.py. The method exists, is correctly imported, and follows the same usage pattern found throughout the codebase (e.g., inuser.pyanduser_template.py).
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.