refactor(integrations): split monolithic module into separate discord and twitch modules#179
refactor(integrations): split monolithic module into separate discord and twitch modules#179danielhe4rt merged 1 commit into4.xfrom
Conversation
… and twitch modules - Create separate app-modules/integration-discord and app-modules/integration-twitch modules - Move Discord OAuth implementation to dedicated module with proper namespace - Move Twitch OAuth and subscribers implementation to dedicated module - Update IdentityProvider enum imports to reference new module namespaces - Remove monolithic integrations module in favor of modular structure - Update composer.json to require individual integration modules - Add service providers for each integration module with appropriate bindings
📝 WalkthroughWalkthroughThis pull request refactors the integration modules from a monolithic structure to separate, modular components. The previous 🚥 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. 📝 Coding Plan
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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
app-modules/integration-discord/composer.json (1)
3-3: Consider adding a non-empty package description.Line 3 currently sets
"description": ""; adding a short description improves package metadata quality and discoverability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-discord/composer.json` at line 3, The package metadata has an empty description field ("description": "") in composer.json; update the "description" value to a concise non-empty string that summarizes the package (e.g., one short sentence describing the Discord integration), ensuring the key remains "description" and the string is properly quoted in composer.json.app-modules/integration-discord/src/Providers/IntegrationDiscordServiceProvider.php (1)
11-13: Wire Discord client in the provider (or remove the empty scaffold).
register()andboot()are both empty, so this provider currently adds no container behavior. Consider bindingDiscordOAuthClienthere for consistency with the Twitch module wiring and easier DI testing.Suggested refactor
use Illuminate\Support\ServiceProvider; +use He4rt\IntegrationDiscord\OAuth\DiscordOAuthClient; class IntegrationDiscordServiceProvider extends ServiceProvider { - public function register(): void {} + public function register(): void + { + $this->app->singleton(DiscordOAuthClient::class); + } public function boot(): void {} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-discord/src/Providers/IntegrationDiscordServiceProvider.php` around lines 11 - 13, The IntegrationDiscordServiceProvider currently has empty register() and boot() methods; either remove the unused provider or wire the Discord client into the container: in IntegrationDiscordServiceProvider::register(), bind or singleton the DiscordOAuthClient (the same way Twitch wiring does) so consumers can type-hint DiscordOAuthClient for DI and tests can swap a mock; keep boot() only if you need runtime setup (otherwise leave it empty or remove it) and ensure the binding references the concrete class name DiscordOAuthClient and any required factory/config values.app-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.php (1)
14-15: Prefer contract-based resolution for both providers.Line 14 imports a concrete
DiscordOAuthClientwhile Line 15 uses a Twitch contract. Consider introducing/resolving a Discord contract too, soIdentityProviderdepends only on abstractions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.php` around lines 14 - 15, The IdentityProvider currently imports and depends on the concrete DiscordOAuthClient while using the TwitchOAuthService contract; change IdentityProvider to depend on an abstraction for Discord as well: introduce or use a DiscordOAuthService interface (nameable as DiscordOAuthService) and replace the concrete import/typing of DiscordOAuthClient in IdentityProvider with that interface, update any constructor/type hints to the interface, and ensure the DI container is bound to resolve DiscordOAuthClient as the implementation for DiscordOAuthService so both providers are resolved via contracts.app-modules/integration-twitch/composer.json (1)
8-15: Move test namespace toautoload-devto avoid loading test classes in production.The
Testsnamespace is currently in the mainautoloadsection, which means test classes will be autoloaded in production environments. This increases the autoloader size unnecessarily and could expose test code.♻️ Proposed fix
"autoload": { "psr-4": { "He4rt\\IntegrationTwitch\\": "src/", - "He4rt\\IntegrationTwitch\\Tests\\": "tests/", "He4rt\\IntegrationTwitch\\Database\\Factories\\": "database/factories/", "He4rt\\IntegrationTwitch\\Database\\Seeders\\": "database/seeders/" } - }, + }, + "autoload-dev": { + "psr-4": { + "He4rt\\IntegrationTwitch\\Tests\\": "tests/" + } + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-twitch/composer.json` around lines 8 - 15, The composer.json currently lists the test namespace under "autoload" which causes test classes (He4rt\\IntegrationTwitch\\Tests\\) to be loaded in production; move the "He4rt\\IntegrationTwitch\\Tests\\" entry from the "autoload" -> "psr-4" block into an "autoload-dev" -> "psr-4" block so test classes are only autoloaded in development, leaving the other entries (He4rt\\IntegrationTwitch\\, Database\\Factories\\, Database\\Seeders\\) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.php`:
- Around line 14-15: The IdentityProvider currently imports and depends on the
concrete DiscordOAuthClient while using the TwitchOAuthService contract; change
IdentityProvider to depend on an abstraction for Discord as well: introduce or
use a DiscordOAuthService interface (nameable as DiscordOAuthService) and
replace the concrete import/typing of DiscordOAuthClient in IdentityProvider
with that interface, update any constructor/type hints to the interface, and
ensure the DI container is bound to resolve DiscordOAuthClient as the
implementation for DiscordOAuthService so both providers are resolved via
contracts.
In `@app-modules/integration-discord/composer.json`:
- Line 3: The package metadata has an empty description field ("description":
"") in composer.json; update the "description" value to a concise non-empty
string that summarizes the package (e.g., one short sentence describing the
Discord integration), ensuring the key remains "description" and the string is
properly quoted in composer.json.
In
`@app-modules/integration-discord/src/Providers/IntegrationDiscordServiceProvider.php`:
- Around line 11-13: The IntegrationDiscordServiceProvider currently has empty
register() and boot() methods; either remove the unused provider or wire the
Discord client into the container: in
IntegrationDiscordServiceProvider::register(), bind or singleton the
DiscordOAuthClient (the same way Twitch wiring does) so consumers can type-hint
DiscordOAuthClient for DI and tests can swap a mock; keep boot() only if you
need runtime setup (otherwise leave it empty or remove it) and ensure the
binding references the concrete class name DiscordOAuthClient and any required
factory/config values.
In `@app-modules/integration-twitch/composer.json`:
- Around line 8-15: The composer.json currently lists the test namespace under
"autoload" which causes test classes (He4rt\\IntegrationTwitch\\Tests\\) to be
loaded in production; move the "He4rt\\IntegrationTwitch\\Tests\\" entry from
the "autoload" -> "psr-4" block into an "autoload-dev" -> "psr-4" block so test
classes are only autoloaded in development, leaving the other entries
(He4rt\\IntegrationTwitch\\, Database\\Factories\\, Database\\Seeders\\)
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7063b63f-af9e-4a0c-923d-206428c45b5c
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
app-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.phpapp-modules/integration-discord/composer.jsonapp-modules/integration-discord/database/factories/.gitkeepapp-modules/integration-discord/database/migrations/.gitkeepapp-modules/integration-discord/database/seeders/.gitkeepapp-modules/integration-discord/src/OAuth/DiscordOAuthAccessDTO.phpapp-modules/integration-discord/src/OAuth/DiscordOAuthClient.phpapp-modules/integration-discord/src/OAuth/DiscordOAuthUser.phpapp-modules/integration-discord/src/Providers/IntegrationDiscordServiceProvider.phpapp-modules/integration-twitch/composer.jsonapp-modules/integration-twitch/database/factories/.gitkeepapp-modules/integration-twitch/database/migrations/.gitkeepapp-modules/integration-twitch/database/seeders/.gitkeepapp-modules/integration-twitch/src/Client/TwitchBaseClient.phpapp-modules/integration-twitch/src/Contracts/TwitchService.phpapp-modules/integration-twitch/src/OAuth/Client/TwitchOAuthClient.phpapp-modules/integration-twitch/src/OAuth/Contracts/TwitchOAuthService.phpapp-modules/integration-twitch/src/OAuth/DTO/TwitchOAuthAccessDTO.phpapp-modules/integration-twitch/src/OAuth/DTO/TwitchOAuthDTO.phpapp-modules/integration-twitch/src/Providers/IntegrationTwitchServiceProvider.phpapp-modules/integration-twitch/src/Subscriber/Client/TwitchSubscribersClient.phpapp-modules/integration-twitch/src/Subscriber/Contracts/TwitchSubscribersService.phpapp-modules/integration-twitch/src/Subscriber/DTO/TwitchSubscriberDTO.phpapp-modules/integration-twitch/src/Subscriber/Enum/SubscriptionTiersEnum.phpapp-modules/integrations/composer.jsonapp-modules/integrations/phpstan.ignore.neonapp-modules/integrations/phpstan.neonapp-modules/integrations/src/Common/Contracts/TwitchService.phpapp-modules/integrations/src/Providers/IntegrationsServiceProvider.phpcomposer.json
💤 Files with no reviewable changes (5)
- app-modules/integrations/src/Common/Contracts/TwitchService.php
- app-modules/integrations/src/Providers/IntegrationsServiceProvider.php
- app-modules/integrations/phpstan.ignore.neon
- app-modules/integrations/phpstan.neon
- app-modules/integrations/composer.json
Summary
integrationsmodule into two dedicated modules:integration-discordandintegration-twitchIdentityProviderenum imports to reference new module namespacesintegrationsmoduleChanges
composer.jsonto require individual integration modulesTesting
Summary by CodeRabbit