-
Notifications
You must be signed in to change notification settings - Fork 0
Adding of multi-domain settings #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Bumps [cryptography](https://github.com/pyca/cryptography) from 45.0.7 to 46.0.1. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](pyca/cryptography@45.0.7...46.0.1) --- updated-dependencies: - dependency-name: cryptography dependency-version: 46.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…-46.0.1 Bump cryptography from 45.0.7 to 46.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements multi-domain support for DirectAdmin email forwarder management, replacing the single domain configuration with a comprehensive domain management system.
- Adds a new database model and migration system for managing multiple domains per user
- Introduces domain selection interface in the dashboard to switch between domains
- Updates settings page to include domain management UI with add/remove/reorder functionality
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| app/models.py | Adds UserDomain model and domain management methods to User model |
| app/main.py | Updates API endpoints to support domain-specific operations and auto-migration |
| app/settings.py | Adds domain management API endpoints and updates connection testing |
| app/directadmin_api.py | Enhances API parsing and adds domain validation functionality |
| app/templates/settings.html | Replaces single domain field with domain management interface |
| app/templates/dashboard.html | Adds domain selector dropdown |
| static/settings.js | Implements domain management UI functionality |
| static/dashboard.js | Updates to load and switch between domains |
| static/style.css | Adds CSS for domain management components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| async function testConnection(event) { | ||
| console.log('testConnection called with event:', event); | ||
|
|
||
| if (!event) { | ||
| console.error('No event passed to testConnection'); | ||
| alert('Error: No event object passed to testConnection function'); | ||
| return; | ||
| } |
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The testConnection function has excessive console logging and debugging code that appears to be left from development. This should be cleaned up for production by removing or reducing the debug statements.
| console.log('Resetting button from:', testButton.textContent, 'to:', originalText); | ||
| testButton.textContent = originalText; | ||
| testButton.disabled = false; | ||
| console.log('Button reset complete:', testButton.textContent, 'disabled:', testButton.disabled); |
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resetButton function contains verbose debug logging that should be removed for production code.
| console.log('Resetting button from:', testButton.textContent, 'to:', originalText); | |
| testButton.textContent = originalText; | |
| testButton.disabled = false; | |
| console.log('Button reset complete:', testButton.textContent, 'disabled:', testButton.disabled); | |
| testButton.textContent = originalText; | |
| testButton.disabled = false; | |
| console.log('testConnection finally block executing...'); | ||
|
|
||
| // Multiple attempts to reset the button | ||
| try { | ||
| resetButton(); | ||
| console.log('testConnection completed, button reset successfully'); | ||
| } catch (resetError) { | ||
| console.error('Error resetting button with resetButton():', resetError); | ||
| } | ||
|
|
||
| // Always try direct button reset as backup | ||
| try { | ||
| console.log('Attempting direct button reset...'); | ||
| testButton.textContent = originalText || 'Test Connection'; | ||
| testButton.disabled = false; | ||
| console.log('Direct button reset completed'); | ||
| } catch (directResetError) { | ||
| console.error('Error with direct button reset:', directResetError); | ||
| } | ||
|
|
||
| // Ultimate fallback - find button by ID and reset | ||
| try { | ||
| const btn = document.getElementById('test-connection-btn'); | ||
| if (btn) { | ||
| console.log('Ultimate fallback: resetting button by ID'); | ||
| btn.textContent = 'Test Connection'; | ||
| btn.disabled = false; | ||
| } | ||
| } catch (fallbackError) { | ||
| console.error('Ultimate fallback failed:', fallbackError); | ||
| } | ||
| } | ||
| } | ||
|
|
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The testConnection function's finally block contains excessive redundancy with multiple fallback mechanisms and debug logging. This overly complex error handling should be simplified to a single, reliable button reset approach.
| console.log('testConnection finally block executing...'); | |
| // Multiple attempts to reset the button | |
| try { | |
| resetButton(); | |
| console.log('testConnection completed, button reset successfully'); | |
| } catch (resetError) { | |
| console.error('Error resetting button with resetButton():', resetError); | |
| } | |
| // Always try direct button reset as backup | |
| try { | |
| console.log('Attempting direct button reset...'); | |
| testButton.textContent = originalText || 'Test Connection'; | |
| testButton.disabled = false; | |
| console.log('Direct button reset completed'); | |
| } catch (directResetError) { | |
| console.error('Error with direct button reset:', directResetError); | |
| } | |
| // Ultimate fallback - find button by ID and reset | |
| try { | |
| const btn = document.getElementById('test-connection-btn'); | |
| if (btn) { | |
| console.log('Ultimate fallback: resetting button by ID'); | |
| btn.textContent = 'Test Connection'; | |
| btn.disabled = false; | |
| } | |
| } catch (fallbackError) { | |
| console.error('Ultimate fallback failed:', fallbackError); | |
| } | |
| } | |
| } | |
| resetButton(); | |
| } | |
| } | |
| // Helper to reliably reset the test connection button | |
| function resetButton() { | |
| const btn = document.getElementById('test-connection-btn'); | |
| if (btn) { | |
| btn.textContent = 'Test Connection'; | |
| btn.disabled = false; | |
| } | |
| } |
| if data: | ||
| print(f"Request data: {data}") | ||
|
|
||
| print(f"Starting HTTP request with 10 second timeout...") |
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug print statements should be removed from production code or replaced with proper logging.
| # Ensure DB initialization only once (important with multi-worker if --preload not used) | ||
| if not app.config.get('_DB_INITIALIZED', False): | ||
| with app.app_context(): | ||
| print(f"Initializing database at URI: {app.config['SQLALCHEMY_DATABASE_URI']}") |
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Database initialization contains numerous print statements for debugging that should be replaced with proper logging or removed for production.
Add adding of multiple domains in your user settings and a dashboard option to switch between them