Add Sentry alert rule creation functionality - #569
Merged
Merged
Conversation
- Implement `createIssueAlertRule` in `SentryClient` to create Sentry issue alert rules. - Add `ensureAlertRuleConfigured` for Sentry rule prerequisites validation. - Extend `ConnectorsAndSyncSettings` with a new action to create Sentry alert rules via user inputs. - Add feature tests for creating Sentry alert rules and validating API requests.
Contributor
Reviewer's GuideImplements end-to-end support for creating Sentry issue alert rules targeting the Forge Sentry app, including a new Filament UI action, SentryClient helper with configuration validation, and feature tests that verify the outbound Sentry API request shape. Sequence diagram for creating a Sentry issue alert rule from the UIsequenceDiagram
actor Admin
participant ConnectorsAndSyncSettings
participant SentryClient
participant SentryAPI
Admin->>ConnectorsAndSyncSettings: Action createSentryAlertRule(data)
ConnectorsAndSyncSettings->>ConnectorsAndSyncSettings: createSentryAlertRule(data)
ConnectorsAndSyncSettings->>SentryClient: createIssueAlertRule(sentryProjectSlug, ruleName, forgeProjectId, issueTypeId, priorityId, frequency)
SentryClient->>SentryClient: ensureAlertRuleConfigured()
SentryClient->>SentryAPI: POST /projects/{org_slug}/{project_slug}/rules/
SentryAPI-->>SentryClient: 200 OK, rule JSON
SentryClient-->>ConnectorsAndSyncSettings: rule
ConnectorsAndSyncSettings-->>Admin: Success notification
alt error
SentryAPI-->>SentryClient: error response
SentryClient-->>ConnectorsAndSyncSettings: throws ConnectionException
ConnectorsAndSyncSettings-->>Admin: Error notification
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Owner
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
ConnectorsAndSyncSettings::createSentryAlertRule, you fall back to default values and cast for all inputs before callingSentryClient; consider validating and failing early when required fields like project/issue type/priority are missing instead of silently sending empty IDs to Sentry. - The error notification in
createSentryAlertRuleuses$e->getMessage()directly in the UI, which may expose internal details from Sentry or the integration layer; consider logging the exception and showing a more user-friendly, generic error message instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ConnectorsAndSyncSettings::createSentryAlertRule`, you fall back to default values and cast for all inputs before calling `SentryClient`; consider validating and failing early when required fields like project/issue type/priority are missing instead of silently sending empty IDs to Sentry.
- The error notification in `createSentryAlertRule` uses `$e->getMessage()` directly in the UI, which may expose internal details from Sentry or the integration layer; consider logging the exception and showing a more user-friendly, generic error message instead.
## Individual Comments
### Comment 1
<location path="app/Filament/Pages/ConnectorsAndSyncSettings.php" line_range="337-340" />
<code_context>
+ ->body($ruleId !== '' ? 'Rule ID: '.$ruleId : 'Sentry accepted the rule.')
+ ->success()
+ ->send();
+ } catch (\Throwable $e) {
+ Notification::make()
+ ->title('Could not create Sentry rule')
+ ->body($e->getMessage())
+ ->danger()
+ ->send();
</code_context>
<issue_to_address>
**🚨 issue (security):** Catching Throwable and surfacing the raw exception message to the user can leak internal details.
Using `$e->getMessage()` directly in a user notification may expose sensitive implementation or configuration details. Instead, log the full exception (with stack trace) to your logging/Sentry pipeline, and present a generic, user-friendly error message—optionally including a correlation ID for support/debugging.
</issue_to_address>
### Comment 2
<location path="app/Filament/Pages/ConnectorsAndSyncSettings.php" line_range="199-203" />
<code_context>
+ ->label('Rule name')
+ ->default('Send new issues to Forge')
+ ->required(),
+ TextInput::make('frequency')
+ ->label('Action frequency minutes')
+ ->numeric()
+ ->minValue(5)
+ ->maxValue(43200)
+ ->default(5)
+ ->required(),
</code_context>
<issue_to_address>
**suggestion:** Frequency bounds are duplicated here and in SentryClient; consider centralizing to avoid drift.
The `frequency` limits (5, 43200) are enforced here on the form and again in `SentryClient::createIssueAlertRule()` when clamping. Please consider extracting these into shared constants (or deriving one from the other) so that both the UI and client always use the same range.
Suggested implementation:
```
namespace App\Filament\Pages;
use App\Services\SentryClient;
```
```
TextInput::make('frequency')
->label('Action frequency minutes')
->numeric()
->minValue(SentryClient::MIN_FREQUENCY_MINUTES)
->maxValue(SentryClient::MAX_FREQUENCY_MINUTES)
->default(SentryClient::DEFAULT_FREQUENCY_MINUTES)
```
In `App\Services\SentryClient` (or the actual namespace where `SentryClient` lives), define and use shared constants, for example:
```php
class SentryClient
{
public const MIN_FREQUENCY_MINUTES = 5;
public const MAX_FREQUENCY_MINUTES = 43200;
public const DEFAULT_FREQUENCY_MINUTES = self::MIN_FREQUENCY_MINUTES;
public function createIssueAlertRule(array $payload): void
{
$frequency = (int) ($payload['frequency'] ?? self::DEFAULT_FREQUENCY_MINUTES);
$frequency = max(self::MIN_FREQUENCY_MINUTES, min(self::MAX_FREQUENCY_MINUTES, $frequency));
// ...
}
}
```
You may need to adjust:
1. The `use App\Services\SentryClient;` import to match your actual `SentryClient` namespace.
2. The constant names if you prefer a different naming convention, but they should be used both here and in `createIssueAlertRule()` where clamping is currently implemented.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+337
to
+340
| } catch (\Throwable $e) { | ||
| Notification::make() | ||
| ->title('Could not create Sentry rule') | ||
| ->body($e->getMessage()) |
Contributor
There was a problem hiding this comment.
🚨 issue (security): Catching Throwable and surfacing the raw exception message to the user can leak internal details.
Using $e->getMessage() directly in a user notification may expose sensitive implementation or configuration details. Instead, log the full exception (with stack trace) to your logging/Sentry pipeline, and present a generic, user-friendly error message—optionally including a correlation ID for support/debugging.
Comment on lines
+199
to
+203
| TextInput::make('frequency') | ||
| ->label('Action frequency minutes') | ||
| ->numeric() | ||
| ->minValue(5) | ||
| ->maxValue(43200) |
Contributor
There was a problem hiding this comment.
suggestion: Frequency bounds are duplicated here and in SentryClient; consider centralizing to avoid drift.
The frequency limits (5, 43200) are enforced here on the form and again in SentryClient::createIssueAlertRule() when clamping. Please consider extracting these into shared constants (or deriving one from the other) so that both the UI and client always use the same range.
Suggested implementation:
namespace App\Filament\Pages;
use App\Services\SentryClient;
TextInput::make('frequency')
->label('Action frequency minutes')
->numeric()
->minValue(SentryClient::MIN_FREQUENCY_MINUTES)
->maxValue(SentryClient::MAX_FREQUENCY_MINUTES)
->default(SentryClient::DEFAULT_FREQUENCY_MINUTES)
In App\Services\SentryClient (or the actual namespace where SentryClient lives), define and use shared constants, for example:
class SentryClient
{
public const MIN_FREQUENCY_MINUTES = 5;
public const MAX_FREQUENCY_MINUTES = 43200;
public const DEFAULT_FREQUENCY_MINUTES = self::MIN_FREQUENCY_MINUTES;
public function createIssueAlertRule(array $payload): void
{
$frequency = (int) ($payload['frequency'] ?? self::DEFAULT_FREQUENCY_MINUTES);
$frequency = max(self::MIN_FREQUENCY_MINUTES, min(self::MAX_FREQUENCY_MINUTES, $frequency));
// ...
}
}You may need to adjust:
- The
use App\Services\SentryClient;import to match your actualSentryClientnamespace. - The constant names if you prefer a different naming convention, but they should be used both here and in
createIssueAlertRule()where clamping is currently implemented.
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Jun 2, 2026 5:17p.m. | Review ↗ |
| JavaScript | |
Jun 2, 2026 5:17p.m. | Review ↗ |
| Python | |
Jun 2, 2026 5:17p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for creating Sentry issue alert rules that trigger the Forge Sentry app action, including a Filament admin UI entry point, the Sentry API client call, and a feature test to verify the outbound request payload.
Changes:
- Implement
SentryClient::createIssueAlertRule()plus configuration prerequisites (ensureAlertRuleConfigured()). - Add a Filament header action on Connectors & Sync Settings to collect inputs and create the rule.
- Extend Sentry outbound sync feature tests to validate the rule creation request.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/Integrations/Sentry/Services/SentryClient.php | Adds a client method to POST a Sentry issue alert rule payload wired to the Sentry App action, with prerequisite configuration checks. |
| app/Filament/Pages/ConnectorsAndSyncSettings.php | Adds a Filament action + modal form to create the Sentry rule from the admin settings page. |
| tests/Feature/Integrations/Sentry/OutboundSyncTest.php | Adds a feature test validating the outbound request shape for creating an issue alert rule. |
Comment on lines
+322
to
+329
| $rule = app(SentryClient::class)->createIssueAlertRule( | ||
| sentryProjectSlug: trim((string) ($data['sentry_project_slug'] ?? '')), | ||
| ruleName: trim((string) ($data['rule_name'] ?? 'Send new issues to Forge')), | ||
| forgeProjectId: (string) ($data['forge_project_id'] ?? ''), | ||
| issueTypeId: (int) ($data['forge_issue_type_id'] ?? 0), | ||
| priorityId: (int) ($data['forge_priority_id'] ?? 0), | ||
| frequency: (int) ($data['frequency'] ?? 5), | ||
| ); |
Comment on lines
+337
to
+343
| } catch (\Throwable $e) { | ||
| Notification::make() | ||
| ->title('Could not create Sentry rule') | ||
| ->body($e->getMessage()) | ||
| ->danger() | ||
| ->send(); | ||
| } |
Comment on lines
+192
to
+202
| return $request->method() === 'POST' | ||
| && str_ends_with($request->url(), '/projects/acme/crash-game/rules/') | ||
| && $request['conditions'][0]['id'] === 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition' | ||
| && $action['id'] === 'sentry.rules.actions.notify_event_sentry_app.NotifyEventSentryAppAction' | ||
| && $action['sentryAppInstallationUuid'] === '9a89a822-6e0b-4b62-9b99-905b9d742dd1' | ||
| && $action['hasSchemaFormConfig'] === true | ||
| && $action['settings'] === [ | ||
| ['name' => 'forge_project_id', 'value' => (string) $project->id], | ||
| ['name' => 'forge_issue_type_id', 'value' => (string) $type->id], | ||
| ['name' => 'forge_priority_id', 'value' => (string) $priority->id], | ||
| ]; |
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
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.
createIssueAlertRuleinSentryClientto create Sentry issue alert rules.ensureAlertRuleConfiguredfor Sentry rule prerequisites validation.ConnectorsAndSyncSettingswith a new action to create Sentry alert rules via user inputs.Summary by Sourcery
Add UI and backend support for creating Sentry issue alert rules wired to the Forge Sentry app action.
New Features:
Enhancements:
Tests: