forked from nextcloud/mail
-
Notifications
You must be signed in to change notification settings - Fork 0
WIP ionosmail on retry config v2 #26
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
Draft
printminion-co
wants to merge
24
commits into
ionos-dev
Choose a base branch
from
mk/dev/ionosmail-on-retry-config-v2
base: ionos-dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
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
…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]>
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]>
…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]>
…nfig Add immutable withPassword method to create new MailServerConfig instances with updated passwords while preserving other configuration values. This supports password reset scenarios where we need to update credentials without modifying the original configuration object. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…onfig Add immutable withPassword method to create new MailAccountConfig instances with updated passwords for both IMAP and SMTP configurations. This leverages the MailServerConfig.withPassword method added previously. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…Service Add NEXTCLOUD_WORKSPACE constant for consistent app password management across IONOS API calls. This ensures the same application name is used when setting or resetting app passwords. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…object Add immutable result class for conflict resolution scenarios: - retry(): Account can be retried with existing config - noExistingAccount(): No IONOS account exists for conflict resolution - emailMismatch(): Existing account has different email than expected This provides a clean API for handling account creation conflicts. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…ssword reset to IonosMailService Add new methods to IonosMailService: - getAccountConfigForUser: Retrieve existing IONOS account configuration - getAccountConfigForCurrentUser: Retrieve config for logged-in user - resetAppPassword: Reset/regenerate app password for a user - getMailDomain: Expose mail domain from config service Refactor buildSuccessResponse to use shared buildMailAccountConfig helper for consistent configuration building across different response types. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Add service to handle conflict resolution when IONOS account creation fails: - Check if existing IONOS account matches requested email - Reset app password for existing accounts to enable retry - Report email mismatch when existing account has different email This enables retry scenarios where IONOS account was created but Nextcloud account creation failed due to DNS propagation delays. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…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]>
… 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]>
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
8f7ca90 to
5a9053b
Compare
…ovider system This commit introduces a new external mail account provider system for Nextcloud Mail, allowing integration with services like IONOS. It includes the implementation of the IonosProvider, which supports account creation, management, and app password generation. Additionally, it establishes a registry service for managing multiple providers and their capabilities. This enhancement aims to provide users with more flexible options for managing their email accounts directly within Nextcloud. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…pdate mail provider handling This commit removes outdated IONOS-specific routes to streamline the routing configuration. It also updates the handling of mail providers to support a more generic approach, allowing for easier integration of multiple email providers in the future. Signed-off-by: Misha M.-Kupriyanov <[email protected]>
5a9053b to
158efcc
Compare
…duce coupling Introduce IonosProviderFacade to reduce coupling between IonosProvider and IONOS services. This is Phase 1 of the provider architecture refactoring (Option C - Minimal Refactoring). Changes: - Create IonosProviderFacade with unified interface for IONOS operations - Update IonosProvider to use facade instead of 3+ service dependencies - Reduce IonosProvider dependencies by 66% (from 3+ to 1) - Add comprehensive unit tests for IonosProviderFacade (19 test methods) - Simplify IonosProviderTest to use facade mock - Add architecture analysis and refactoring roadmap documentation Benefits: - Improved testability (1 mock vs 3+ mocks in IonosProvider tests) - Reduced coupling between provider and service layers - Clear abstraction boundary for future provider implementations - Foundation for adding new providers (Office365, Google, etc.) Next Phase: Split IonosMailService into focused services Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…cused query and mutation services Split the 557-line IonosMailService into two focused services following Single Responsibility Principle and CQRS pattern. This is Phase 2 of the provider architecture refactoring (Option C - Minimal Refactoring). Changes: - Create IonosAccountQueryService (237 lines) for read operations - Create IonosAccountMutationService (381 lines) for write operations - Update IonosProviderFacade to use new split services - Update IonosProviderFacadeTest to use new service mocks - Achieve 32-57% reduction in service size Benefits: - Clear separation of concerns (CQRS pattern: Command/Query separation) - Single Responsibility Principle applied (read vs write) - Reduced cognitive complexity (smaller, focused services) - Better testability (isolated concerns) - Easier maintenance (clear boundaries) Query Service (Read Operations): - mailAccountExistsForCurrentUser/UserId - getMailAccountResponse - getAccountConfigForUser/CurrentUser - getIonosEmailForUser - getMailDomain Mutation Service (Write Operations): - createEmailAccount/ForUser - deleteEmailAccount - tryDeleteEmailAccount - resetAppPassword Test Results: - IonosProviderFacadeTest: 19 tests, 47 assertions ✅ - All Provider Tests: 53 tests, 114 assertions ✅ Files Created: - lib/Service/IONOS/Core/IonosAccountQueryService.php (237 lines) - lib/Service/IONOS/Core/IonosAccountMutationService.php (381 lines) Files Modified: - lib/Provider/MailAccountProvider/Implementations/Ionos/IonosProviderFacade.php - tests/Unit/Provider/MailAccountProvider/Implementations/Ionos/IonosProviderFacadeTest.php Next Phase: Move DTOs to shared location Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…n for reusability Move MailAccountConfig and MailServerConfig DTOs from IONOS-specific location to shared Common location, making them reusable by future providers (Office365, Google, etc.). This is Phase 3 of the provider architecture refactoring (Option C - Minimal Refactoring). Changes: - Create lib/Provider/MailAccountProvider/Common/Dto/ directory - Move MailServerConfig to Common/Dto with updated namespace - Move MailAccountConfig to Common/Dto with updated namespace - Update all 5 import statements across IONOS services - Enable 100% reusability across all future providers Benefits: - DTOs are now provider-agnostic and fully reusable - Clear namespace indicates shared infrastructure - No code duplication needed for future providers - Better organization with clear separation of concerns - Single location to maintain and extend DTOs Before (IONOS-specific): - Namespace: OCA\Mail\Service\IONOS\Dto - Location: lib/Service/IONOS/Dto/ - Usage: IONOS only After (Shared): - Namespace: OCA\Mail\Provider\MailAccountProvider\Common\Dto - Location: lib/Provider/MailAccountProvider/Common/Dto/ - Usage: All providers (IONOS, Office365, Google, etc.) Files Updated: - IonosAccountQueryService - IonosAccountMutationService - IonosAccountCreationService - ConflictResolutionResult - IonosMailService Test Results: - All Provider Tests: 53 tests, 114 assertions ✅ Files Created: - lib/Provider/MailAccountProvider/Common/Dto/MailServerConfig.php - lib/Provider/MailAccountProvider/Common/Dto/MailAccountConfig.php Next Phase: Documentation and polish Signed-off-by: Misha M.-Kupriyanov <[email protected]>
b1d01a5 to
f0fde12
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.

No description provided.