-
Notifications
You must be signed in to change notification settings - Fork 0
ionosmail on retry config #21
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
e38c671 to
79c50cf
Compare
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 pull request implements retry logic for IONOS mail account creation when API failures occur. When account creation initially fails, the system now attempts to resolve conflicts by checking if an IONOS account already exists for the user, and if so, retrieves a fresh password to retry the Nextcloud account creation process.
Key Changes:
- Added conflict resolution mechanism to handle cases where IONOS API account exists but Nextcloud account creation failed
- Implemented password reset functionality to retrieve new credentials for existing IONOS accounts
- Added CLI command for creating IONOS accounts outside of web requests
- Introduced skip connectivity test option to allow account creation when IONOS has already validated credentials
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
lib/Service/IONOS/IonosAccountConflictResolver.php |
New service to resolve conflicts when account creation fails by checking for existing accounts |
lib/Service/IONOS/ConflictResolutionResult.php |
DTO representing the result of conflict resolution with retry capability |
lib/Service/IONOS/IonosMailService.php |
Added methods to get existing account config, reset app passwords, and create accounts for specific users |
lib/Service/IONOS/Dto/MailAccountConfig.php |
Added withPassword() method to create new config instances with updated passwords |
lib/Service/IONOS/Dto/MailServerConfig.php |
Added withPassword() method to create new config instances with updated passwords |
lib/Controller/IonosAccountsController.php |
Integrated conflict resolver to retry account creation when initial attempt fails |
lib/Controller/AccountsController.php |
Added skipConnectivityTest parameter to bypass connection testing |
lib/Service/SetupService.php |
Added skipConnectivityTest parameter to allow account creation without connectivity validation |
lib/Command/IonosCreateAccount.php |
New CLI command for creating IONOS accounts via OCC |
tests/Unit/Service/IONOS/IonosMailServiceTest.php |
Comprehensive test coverage for new mail service methods |
tests/Unit/Service/IONOS/IonosAccountConflictResolverTest.php |
Test coverage for conflict resolution logic |
tests/Unit/Controller/IonosAccountsControllerTest.php |
Test coverage for retry scenarios in controller |
tests/Unit/Service/SetupServiceTest.php |
New test file for SetupService functionality |
tests/Unit/Command/IonosCreateAccountTest.php |
Test coverage for CLI command |
composer.json |
Updated IONOS API client reference |
appinfo/info.xml |
Registered new CLI command |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
79c50cf to
7c42154
Compare
…251208083401 composer update ionos-productivity/ionos-mail-configuration-api-client https://github.com/IONOS-Productivity/ionos-mail-configuration-api-client/releases/tag/2.0.0-20251208083401 Signed-off-by: Misha M.-Kupriyanov <[email protected]>
7c42154 to
6d36dee
Compare
Added comprehensive unit tests for the SetupService class, covering account creation with various authentication methods, handling of connectivity tests, and validation of authentication methods. This enhances test coverage and ensures the reliability of the account setup functionality. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
6d36dee to
7219b90
Compare
…o createNewAccount Add optional parameter to allow skipping IMAP/SMTP connectivity tests during account creation. This is useful when account credentials are already validated by external systems or when immediate connectivity cannot be guaranteed due to DNS propagation delays. The parameter defaults to false to maintain backward compatibility. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…eter to create method Add optional parameter to AccountsController::create() to allow skipping IMAP/SMTP connectivity tests during account creation. This parameter is passed through to SetupService::createNewAccount(). This enables external systems (e.g., IONOS API) that have already validated credentials to skip redundant connectivity tests, particularly useful when DNS propagation delays might cause immediate connectivity tests to fail. The parameter defaults to false to maintain backward compatibility.
… data and improved mock setups Refactor test methods to utilize constants for user and email details, ensuring consistency and readability. This change improves maintainability and reduces duplication across test cases. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…nt for specific user This change introduces a new method to create an IONOS email account for a specified user, allowing for account creation without relying on the user session. This is particularly useful for OCC commands or administrative operations. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
b242311 to
fb86530
Compare
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
Copilot reviewed 26 out of 27 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw new ServiceException('Failed to create ionos mail', $result->getStatus()); | ||
| } | ||
| if ($result instanceof MailAccountResponse) { | ||
| if ($result instanceof MailAccountCreatedResponse) { |
Copilot
AI
Dec 16, 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 comment on line 318 states this method handles MailAccountCreatedResponse from createFunctionalAccount, but the actual API call on line 168 uses createMailbox, not createFunctionalAccount. Update the comment to accurately reflect the API method being called.
1b7629d to
955ec81
Compare
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
Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| use GuzzleHttp\ClientInterface; | ||
| use IONOS\MailConfigurationAPI\Client\Api\MailConfigurationAPIApi; | ||
| use IONOS\MailConfigurationAPI\Client\Model\Imap; | ||
| use IONOS\MailConfigurationAPI\Client\Model\MailAccountCreatedResponse; |
Copilot
AI
Dec 17, 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 import for MailAccountCreatedResponse is added, but verify that test coverage exists for all methods that use this type versus MailAccountResponse. Ensure there are tests verifying the distinction between these response types in account creation vs retrieval scenarios.
|
|
||
| // Get fresh password via resetAppPassword API since getAccountConfigForUser | ||
| // does not return password for security reasons | ||
| $newPassword = $this->ionosMailService->resetAppPassword($userId, IonosConfigService::APP_NAME); |
Copilot
AI
Dec 17, 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.
Add a comment explaining that resetAppPassword generates a new app-specific password from the IONOS API, as this is a critical security operation and the behavior may not be immediately obvious from the method name alone.
955ec81 to
10abdfd
Compare
…anced error handling Introduce IonosServiceException to provide a structured way to handle exceptions related to IONOS services, including additional data storage for context. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Add unified service for creating and updating IONOS mail accounts: - Check for existing Nextcloud accounts before creation - Handle conflict resolution with existing IONOS accounts - Create new accounts with encrypted credentials - Update existing accounts with fresh credentials This service provides consistent account creation logic for both CLI and web interfaces, with proper retry handling for DNS propagation delays. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…availability check Update isMailConfigAvailable to check for local account configuration: - Add AccountService and IUserSession dependencies - Check if user has remote IONOS account - Check if remote account is configured locally in mail app - Show configuration if remote exists but local doesn't (retry scenario) This allows users to complete account setup if the initial Nextcloud account creation failed but the IONOS account was created. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…ogging in IonosAccountsController Enhanced error handling by introducing a dedicated method for building service error responses. Improved logging messages for better clarity during account creation process. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…count creation Enhances the IonosAccountsController by integrating user session management, allowing for user identification during email account creation. This improves error handling by ensuring that a valid user session is present, providing more informative logging and responses in case of session absence. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…ice in IonosAccountsController Refactor IonosAccountsController to use the shared IonosAccountCreationService: - Replace IonosMailService and AccountsController with IonosAccountCreationService - Add IUserSession for user ID retrieval - Simplify create method by delegating to IonosAccountCreationService - Add error message to service exception responses This ensures consistent account creation logic between web and CLI interfaces. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
10abdfd to
41309ff
Compare
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
Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| use IONOS\MailConfigurationAPI\Client\ApiException; | ||
| use IONOS\MailConfigurationAPI\Client\Model\Imap; | ||
| use IONOS\MailConfigurationAPI\Client\Model\MailAccountCreatedResponse; | ||
| use IONOS\MailConfigurationAPI\Client\Model\MailAccountResponse; |
Copilot
AI
Dec 17, 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 import for MailAccountResponse at line 16 appears to be used only in private methods that handle existing accounts. Consider documenting in these methods that MailAccountResponse is the type returned by getFunctionalAccount (for existing accounts) while MailAccountCreatedResponse is returned by createFunctionalAccount (for new accounts) to clarify the distinction between these similar types.
… existing email address conflict Updated the feedback message for the 409 error code to provide clearer guidance on existing IONOS email addresses, including the conflicting email address. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
09628c7 to
9e436c2
Compare
9e436c2 to
d4022b9
Compare
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
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
npm run buildin order to rebuld the frontend codeTests
npm run buildin order to rebuld the frontend code./occ config:app:set --value true --type boolean -- mail ionos_mailconfig_api_allow_insecureg1user2in nextcloud../occ mail:account:delete $ACCOUNT_IDKnown issues