Repository navigation
Extract Slack notifications and preserve queued transport APIs - #650
Conversation
Declare hypervel/slack-notification-channel as an optional package with its direct runtime dependencies, standard license, and upstream documentation links. Wire its shared Notifications namespace and provider discovery into monorepo development. The Slack provider registers the channel through ChannelManager and owns the fixed slack-notifications HTTP connection; it does not add a core default provider or redundant service bindings.
…kage Move the Slack message contracts, route value object, and event metadata out of core Notifications while preserving their public namespaces and behavior. Keep these shared types beside the channel and builders that use them. Their names and layout continue to follow the upstream Slack notification package.
Move the confirmation object into the Slack channel package without changing its public API or payload behavior. Keep its existing validation and payload tests under the upstream unit-test layout.
Move the plain-text object into the Slack channel package while retaining its length validation, emoji handling, and payload shape. Relocate its existing tests into the upstream unit-test layout.
Move the text object into the Slack channel package without changing its plain-text or markdown APIs. Retain the existing tests for payload formatting and validation.
Move action blocks and their tests into the Slack channel package. Filter optional fields by absence rather than truthiness so a block ID of 0 is retained. Keep the upstream block structure and existing element validation, and cover the meaningful zero-valued identifier alongside the retained payload tests.
Move context blocks and their tests into the Slack channel package. Preserve a block ID of 0 when formatting optional fields. Correct the element annotation to include TextObject, which is added by the text methods alongside element-contract implementations. This changes the annotation without narrowing the public extension surface.
Move divider blocks and their tests into the Slack channel package. Preserve an explicitly supplied block ID of 0 while continuing to omit absent optional fields.
Move header blocks and their tests into the Slack channel package. Retain the existing text validation and formatting, and preserve an explicitly supplied block ID of 0.
Move image blocks and their tests into the Slack channel package. Preserve a block ID of 0 while retaining the existing image, title, and alternative-text payload behavior.
Move section blocks and their tests into the Slack channel package. Preserve a block ID of 0 while continuing to omit absent values and empty field lists. Retain the existing text, accessory, and field validation and payload coverage.
Move button elements and their tests into the Slack channel package. Preserve value and accessibility-label strings containing 0 instead of dropping them through truthy filtering. Keep the upstream button API, validation, and payload structure, with coverage for these valid optional strings.
Move image elements into the Slack channel package without changing their public API or payload shape. Retain the existing validation and serialization tests under the upstream unit-test layout.
Move the select-element base class and generated-ID trait beside their consumers in the Slack channel package. Preserve the upstream nested Elements/Traits layout and the existing shared select behavior. No new identifier mechanism or compatibility alias is introduced.
Move this select type into the Slack channel package while preserving its public API, validation, and payload behavior. Retain its existing regression coverage under the upstream unit-test layout so future upstream changes can be compared directly.
Move this select type into the Slack channel package while preserving its public API, validation, and payload behavior. Retain its existing regression coverage under the upstream unit-test layout so future upstream changes can be compared directly.
Move this select type into the Slack channel package while preserving its public API, validation, and payload behavior. Retain its existing regression coverage under the upstream unit-test layout so future upstream changes can be compared directly.
Move the Block Kit Slack message builder into the Slack channel package without changing its message API or payload behavior. Move its feature tests, named notification fixtures, and shared testbench setup with it. Keep the existing builder coverage and use the package provider when exercising delivery through the framework.
Move the webhook message and attachment builders into the Slack channel package while preserving their public namespaces. Use nullable defaults for optional unfurl flags and timestamps so unset values remain distinct from explicit false or epoch-zero values. Allow the webhook channel setter to accept null, which lets a webhook use its configured default channel without changing existing string calls.
Move SlackWebhookChannel into the optional Slack channel package and retain its Guzzle transport and per-message HTTP options. Filter optional payload fields by absence rather than truthiness so explicit false unfurl flags, zero-valued strings, and epoch-zero attachment timestamps are retained. Continue to omit empty optional arrays and unset values, and retain the no-route early return. Move the existing webhook regression suite with the channel and extend coverage for these payload cases, default omission, and null-channel routing.
Move the modern API channel to Slack/SlackChannel, matching the upstream name and namespace. Use the HTTP factory and the slack-notifications connection for JSON requests, bearer tokens, normal HTTP errors, and standard HTTP response APIs. Keep route and payload resolution per send so connection reuse does not retain recipient state. Preserve Slack API error handling and remove the redundant notification-method guard. Move and extend the channel tests for routing, token isolation, fake delivery, HTTP failures, and Slack API failures.
Move SlackNotificationRouterChannel to the upstream root namespace and retain the container contract used to resolve its channels. Keep URL and URI routes on the webhook channel, other routes on the modern API channel, and false routes disabled. Remove the core Slack driver factory so discovery of the optional package owns registration. Move the router tests and verify that core Notifications alone does not provide the Slack driver.
Declare the HTTP response, Guzzle message and transfer-stat, and serializable-closure dependencies used directly by notification-event serialization. Move the Slack-only mbstring requirement and Slack-specific README material out of core Notifications. Keep package metadata tests aligned with the actual direct dependencies and the core notification provider.
Queued terminal notification events can contain HTTP responses or exceptions with body streams that PHP cannot serialize. A successful delivery can then fail while queueing its listener, and a failure listener can replace the original delivery error. Prepare recognized transport state only at the notification-event serialization boundary and restore the normal response and exception classes before listeners run. Preserve model restoration, direct shared transport references, subclass state, request targets, headers, buffer state, and previous exceptions. Keep native custom serialization contracts under PHP control, including user-defined wakeup hooks. Preserve default writable memory buffers and closed or detached built-in streams; reject unsupported body types without consuming them. Omit trace argument values only in prepared queued exception copies, leaving the original exception unchanged. Cover actual serializing listener jobs, channel success and failure events, sensitive arguments, native hook ordering, buffer boundaries, and database model restoration. Generic queue serialization and existing Symfony mail exception handling remain unchanged.
Declare Horizon's direct dependency on hypervel/slack-notification-channel now that the builders and delivery channels live outside core Notifications. Verify the package dependency and the supported webhook route without an explicit channel. The regression confirms Horizon retains its attachment payload while allowing the webhook's configured channel default.
Document installation of hypervel/slack-notification-channel and its automatically registered named HTTP connection, including a boot-time timeout configuration example. Explain that queued terminal-event listeners keep the usual response and exception APIs with default HTTP transports. Describe the queued-copy trace argument limitation and give direct guidance for notification values that cannot be serialized, without exposing the internal snapshot implementation.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (78)
💤 Files with no reviewable changes (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds transport-aware serialization for queued notification events and moves Slack notification delivery into a separately registered package. It also updates Slack HTTP handling, payload serialization, Composer metadata, documentation, and related tests. ChangesNotification transport serialization
Slack notification channel
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant NotificationEvent
participant SerializesTransport
participant TransportSnapshot
participant Queue
participant QueuedListener
NotificationEvent->>SerializesTransport: serialize event state
SerializesTransport->>TransportSnapshot: capture response or exception
TransportSnapshot-->>SerializesTransport: return transport snapshot
SerializesTransport->>Queue: provide serialized event state
Queue->>SerializesTransport: restore serialized event state
SerializesTransport->>TransportSnapshot: restore transport object
SerializesTransport->>QueuedListener: provide restored event
Merge Risk: ⚪ Minimal · up to The reviewed changes are mergeable after normal checks; no material notification or Slack delivery failure was established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 44.94% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 178 functions across 50 files. (8 skipped: 8 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@cubic-dev-ai review |
@binaryfire cubic can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 124,545 of the 120,000 allowed lines of code this month. Reviews resume on 10 October 2026 (in 4 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
PR Summary by QodoExtract Slack notifications and preserve queued transport APIs
AI Description
Diagram
High-Level Assessment
Files changed (76)
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Code Review by Qodo
1. Malformed Slack replies look delivered
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
1 participant
Slack notifications currently live inside core Notifications, with a layout and channel names that differ from Laravel's Slack notification package. This makes upstream maintenance harder and makes Slack-specific dependencies part of the core package.
This change extracts Slack into
hypervel/slack-notification-channel, aligns its layout and channel names with upstream, and fixes queued notification events that contain HTTP transport objects.Slack package
Slack\SlackChannel, rootSlackNotificationRouterChannel, andSlackChannelServiceProvidernames.slack-notificationsconnection. Connection handling is shared; tokens, recipients, and payloads remain local to each request.The extraction also preserves optional payload values that truthy filtering previously dropped: explicit
falseunfurl flags, strings containing0, and epoch-zero attachment timestamps. Unset fields and empty optional arrays remain omitted. The webhook message's channel setter acceptsnull, allowing Horizon notifications to use the webhook's default channel.Queued notification events
NotificationDelivered,NotificationSent, andNotificationFailedcan contain HTTP responses or exceptions with body streams. PHP cannot serialize those streams when queueing an event listener. On success, that can fail after Slack has accepted the message and cause a notification retry to send it again. On failure, it can replace the original delivery error with a serialization error.Prepare recognized transport state when the notification event is serialized, then restore the normal response and exception objects before the listener runs. This preserves the usual APIs, model restoration, headers, request targets, previous exceptions, response metadata, and supported buffer state. Synchronous listeners keep the original objects, and generic queue serialization is unchanged.
Objects with custom serialization hooks retain PHP's native behavior, including custom wakeup ordering. Default writable memory buffers and closed or detached built-in streams are supported. Other body types fail clearly without being consumed; the documentation explains how to handle these events synchronously and dispatch a job with only the required data.
Prepared queued exception copies omit stack-trace argument values, which can contain values PHP cannot serialize. The original exception remains unchanged. Existing Symfony mail exception handling is preserved.
Verification
composer lint:fixandcomposer analysepassed, including the committed type fixtures.Note
Extract Slack notifications into
hypervel/slack-notification-channeland preserve queued transport APIshypervel/slack-notification-channelpackage with its own composer manifest,SlackChannelServiceProvider, and MIT license. The notifications package no longer ships aslackdriver; resolvingslackwithout the new package installed now throwsInvalidArgumentException.TransportSnapshotandSerializesTransportso queuedNotificationSent,NotificationDelivered, andNotificationFailedevents serialize HTTP responses, exceptions, and streams and restore the original transport objects on the queued listener side.SlackChannelto use the Hypervel HTTP client factory with a namedslack-notificationsconnection instead of an injected Guzzle client and config repository.falseand0values while still omitting null, empty strings, and empty arrays. Adds regression tests for zero-valuedblock_idand buttonvalue.SlackAttachment.timestampandSlackMessageunfurl properties now default tonullinstead of0/false;SlackWebhookChannel.sendno longer throwsRuntimeExceptionwhen a routed notification lackstoSlack(missing-method error instead); notifications composer.json now requiresguzzlehttp/psr7,hypervel/http, andlaravel/serializable-closureand no longer requiresext-mbstring.📊 Macroscope summarized c5f5580. 27 files reviewed, 2 issues evaluated, 1 issue filtered, 1 comment posted
🗂️ Filtered Issues
src/slack-notification-channel/src/Channels/SlackWebhookChannel.php — 1 comment posted, 2 evaluated, 1 filtered
send()now checks the Slack route before verifying that the notification implementstoSlack. A notification configured for this channel but missingtoSlackwill returnnullwhenever a recipient has no Slack route, whereas the previous implementation raised its explicitRuntimeException; queued/bulk delivery therefore treats this misconfiguration as a successful no-op and loses the diagnostic. [ Out of scope (triage) ]Summary by CodeRabbit