-
Notifications
You must be signed in to change notification settings - Fork 23
update: Add custom domain support with automatic fallback validation #73
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
📝 WalkthroughWalkthroughAdds Custom Domain support to PasteFox Share: new config key, translations, README updates, plugin version bump, settings UI to fetch/choose verified domains via API, persistence of selection, and upload logic that validates API key and uses the custom domain or falls back to pastefox.com. Changes
Sequence DiagramssequenceDiagram
participant User
participant Settings as Plugin Settings
participant API as PasteFox API
participant Env as Environment
participant Upload as UploadLogsAction
User->>Settings: Open settings
Settings->>API: GET /api/domains (X-API-Key if present)
API-->>Settings: Domain list (active/inactive)
Settings->>User: Render domain select (inactive → disabled)
User->>Settings: Select domain & Save
Settings->>Env: Persist PASTEFOX_CUSTOM_DOMAIN
Note over User,Upload: Later — user triggers paste upload
User->>Upload: Trigger upload
Upload->>Env: Read api_key & custom_domain
Upload->>API: Validate API key (isApiKeyValid)
Upload->>API: Verify configured domain (getActiveCustomDomain)
alt custom domain valid
Upload->>API: Create paste using custom domain (X-API-Key header)
else fallback
Upload->>API: Create paste at pastefox.com (no X-API-Key)
end
API-->>Upload: Returns paste URL
Upload-->>User: Show paste URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pastefox-share/src/Filament/Components/Actions/UploadLogsAction.php (2)
149-168: Consider caching API key validation results.Every log upload triggers an HTTP request to validate the API key. For frequently accessed servers, this creates unnecessary load and latency. API keys rarely change, making them good candidates for caching.
Consider using Laravel's cache to store validation results for a reasonable TTL (e.g., 5-10 minutes):
protected function isApiKeyValid(?string $apiKey): bool { if (blank($apiKey)) { return false; } $cacheKey = 'pastefox_api_key_valid_' . hash('sha256', $apiKey); return cache()->remember($cacheKey, 300, function () use ($apiKey) { try { $response = Http::withHeaders([ 'X-API-Key' => $apiKey, 'Content-Type' => 'application/json', ]) ->timeout(5) ->get('https://pastefox.com/api/domains') ->json(); return $response['success'] ?? false; } catch (\Exception $e) { return false; } }); }Note: If you implement the consolidated API call from the previous comment, apply caching to the consolidated method instead.
118-147: Silent failure in domain validation could hide configuration issues.While graceful fallback is appropriate for availability issues, completely silent failure makes it difficult for administrators to diagnose why their custom domain isn't working (e.g., typo in domain name, domain not activated, API connectivity issues).
Consider logging a warning when the configured domain is invalid or inactive:
protected function getActiveCustomDomain(?string $apiKey): ?string { $configuredDomain = config('pastefox-share.custom_domain'); if (blank($configuredDomain) || blank($apiKey)) { return null; } try { $response = Http::withHeaders([ 'X-API-Key' => $apiKey, 'Content-Type' => 'application/json', ]) ->timeout(5) ->get('https://pastefox.com/api/domains') ->json(); if ($response['success'] ?? false) { foreach ($response['domains'] ?? [] as $domain) { if ($domain['domain'] === $configuredDomain && ($domain['isActive'] ?? false)) { return $configuredDomain; } } + // Domain configured but not found or inactive + logger()->warning("PasteFox custom domain '{$configuredDomain}' is not active, falling back to pastefox.com"); } } catch (\Exception $e) { - // Silently fail, fall back to default + logger()->warning("Failed to fetch PasteFox domains: {$e->getMessage()}"); } return null; }This helps administrators identify configuration issues while maintaining the graceful fallback behavior.
pastefox-share/src/PasteFoxSharePlugin.php (1)
104-140: Add PHPDoc return type annotation to satisfy PHPStan.The static analysis error indicates the return type lacks a value type specification. Additionally, the
Content-Typeheader is unnecessary for GET requests.🔎 Proposed fix
+ /** + * @return array<string, string> + */ protected function getCustomDomainOptions(): array { $options = ['' => trans('pastefox-share::messages.custom_domain_none')]; $apiKey = config('pastefox-share.api_key'); if (blank($apiKey)) { return $options; } try { $response = Http::withHeaders([ 'X-API-Key' => $apiKey, - 'Content-Type' => 'application/json', ]) ->timeout(10) ->get('https://pastefox.com/api/domains') ->json();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
pastefox-share/README.mdpastefox-share/config/pastefox-share.phppastefox-share/lang/en/messages.phppastefox-share/plugin.jsonpastefox-share/src/Filament/Components/Actions/UploadLogsAction.phppastefox-share/src/PasteFoxSharePlugin.php
🧰 Additional context used
🪛 GitHub Check: PHPStan
pastefox-share/src/PasteFoxSharePlugin.php
[failure] 104-104:
Method FlexKleks\PasteFoxShare\PasteFoxSharePlugin::getCustomDomainOptions() return type has no value type specified in iterable type array.
🪛 markdownlint-cli2 (0.18.1)
pastefox-share/README.md
24-24: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (7)
pastefox-share/plugin.json (1)
5-5: LGTM! Appropriate version bump.The version increment to 1.1.0 correctly reflects the addition of custom domain support as a new feature.
pastefox-share/config/pastefox-share.php (1)
9-9: LGTM! Configuration entry follows established patterns.The custom domain configuration is consistent with other optional settings in the file.
pastefox-share/lang/en/messages.php (1)
40-49: LGTM! Comprehensive translation coverage.The translation keys are well-structured and provide clear, user-friendly messaging for the custom domain feature.
pastefox-share/README.md (2)
22-29: LGTM! Clear configuration documentation.The updated settings table clearly documents all available configuration options including the new custom domain feature.
38-50: LGTM! Comprehensive custom domain documentation.The documentation clearly explains:
- How to set up custom domains
- The verification process
- Automatic fallback behavior
This provides users with the information needed to configure and use the feature.
pastefox-share/src/PasteFoxSharePlugin.php (2)
87-100: Well-structured conditional UI behavior.The custom domain section properly gates functionality based on API key presence—disabling the select and showing appropriate helper text when no API key is configured. The
disableOptionWhenpattern using the:disabledsuffix cleanly prevents selection of inactive domains while still showing them in the dropdown.
150-150: LGTM!Persistence of the new setting follows the established pattern. Per the AI summary, runtime validation in
UploadLogsActionhandles fallback when a configured domain becomes inactive.
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pastefox-share/src/PasteFoxSharePlugin.php (2)
138-140: Consider logging errors while maintaining graceful fallback.Silent error handling prevents debugging issues with the API integration. Consider logging the exception while still returning default options for graceful degradation.
💡 Suggested refactor to add logging
} catch (\Exception $e) { - // Silently fail, just return default options + \Log::warning('Failed to fetch custom domains from PasteFox API', [ + 'error' => $e->getMessage(), + ]); }
122-122: Consider extracting the API URL to a constant or config.Hardcoding the API URL makes it harder to test or support different environments (staging, local development, etc.).
💡 Suggested refactor
Add to your config file:
'api_base_url' => env('PASTEFOX_API_URL', 'https://pastefox.com'),Then update the code:
- ->get('https://pastefox.com/api/domains') + ->get(config('pastefox-share.api_base_url') . '/api/domains')
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pastefox-share/src/PasteFoxSharePlugin.php
🔇 Additional comments (3)
pastefox-share/src/PasteFoxSharePlugin.php (3)
13-13: LGTM!The Http facade import is correctly added to support the API call in
getCustomDomainOptions().
153-153: LGTM!The persistence of the custom domain setting follows the established pattern and is correctly implemented.
127-136: The logic is not inconsistent. Thestatusfield validates that the domain is registered in the PasteFox system (status === 'ACTIVE'), whileisActiverepresents the user's preference to enable or disable the domain in their dashboard settings. These are complementary validation layers—one system-level, one user-level—not redundant checks.Additionally, the
:disabledsuffix pattern is the proper Filament framework convention. Line 94 shows->disableOptionWhen(fn (string $value): bool => str_ends_with($value, ':disabled')), which is the intended API for presenting disabled options in the select field. This is not a data/UI mixing issue but correct framework integration.Likely an incorrect or invalid review comment.
|
For future reference, please create a separate branch for pull requests and don't use main. |
|
Sorry, I didn't think of that.
…-------- Original Message --------
On Saturday, 01/03/26 at 19:05 Boy132 ***@***.***> wrote:
**Boy132** left a comment (pelican-dev/plugins#73)
For future reference, please create a separate branch for pull requests and
don't use main.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hey, here's a change to my “PasteFox Share” plugin.
Changes
Custom Domains
Users can now select a verified custom domain from their PasteFox account to use for paste URLs. The plugin automatically validates that the domain is still active before using it.
Fallback Behavior
If the configured API key or custom domain becomes unavailable, the plugin gracefully falls back to:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.