-
Notifications
You must be signed in to change notification settings - Fork 128
Feat : Highlight triggered and errored blocks #896
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request updates various components across the API and frontend to enhance the handling, logging, and visual highlighting of blocks during message processing. In the service layer, method parameters are renamed and augmented with error logging and event emissions for both successful and failed highlight scenarios. The websocket gateway now processes these events and relays them to the frontend. Additionally, the visual editor has been extended with new state variables, methods, and event subscriptions to manage node highlighting, error, and fallback states, along with corresponding CSS animations and styling updates. Minor adjustments such as variable renaming, dependency injections, and copyright updates are also included. Changes
Sequence Diagram(s)Highlight Block FlowsequenceDiagram
participant BS as BlockService/BotService
participant EE as Event Emitter
participant WG as WebsocketGateway
participant FE as VisualEditor Frontend
BS->>EE: Emit hook:highlight:block {userId, flowId, blockId, highlightType}
EE->>WG: Deliver hook:highlight:block event
WG->>WG: Invoke handleHighlightBlock
WG->>FE: Emit "highlight:block" via socket with payload
FE->>FE: handleHighlightFlow -> Clear highlights, redirect flow, center block & update highlight state
Assessment against linked issues
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (6)
api/src/chat/services/block.service.ts (1)
571-687
: Consider refactoring repeated error emission logic.While the error emission logic is well-implemented, it's repeated in three different places with nearly identical code. Consider extracting this logic into a helper method to improve maintainability and reduce duplication.
+ /** + * Emits an error highlight event for the block + * + * @param block - The block that encountered an error + * @param recipient - The message recipient + */ + private emitBlockErrorHighlight(block: Block | BlockFull, recipient: Subscriber) { + const flowId = typeof block.category === 'object' + ? block!.category.id + : block.category; + if (flowId) { + this.logger.log('triggered: hook:highlight:error'); + this.eventEmitter.emit('hook:highlight:error', { + flowId, + userId: recipient.foreign_id, + blockId: block.id, + }); + } else { + this.logger.warn('Unable to trigger: hook:highlight:error'); + } + }You could then replace each occurrence with a single call to this method.
🧰 Tools
🪛 Biome (1.9.4)
[error] 618-618: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (2)
41-42
: Using underscore-prefixed properties for private-like fields.
This is a common convention in TypeScript, but consider using real private fields (#myField
) if strict encapsulation is needed.
83-85
: Fire an event when toggling highlight for consistency.
Currently,setHighlighted
does not fire an event, unlikesetHasErrored
. Consider aligning them:setHighlighted(isHighlighted: boolean) { this._isHighlighted = isHighlighted; + this.fireEvent({ isHighlighted }, "stateChanged"); }
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)
275-296
: Remove Promise wrapping in an async function.
SinceremoveHighlights
is already async, you can simplify:- async function removeHighlights() { - return new Promise((resolve) => { - if (!engine) { - return; - } - // ... - engine.repaintCanvas(); - resolve(true); - }); + async function removeHighlights() { + if (!engine) return; + // ... + engine.repaintCanvas(); + return true; }
352-373
: Using setTimeout may be fragile.
Loading and rendering delays might exceed 200ms in some cases. Consider an event-based approach to ensure the block highlight occurs once the diagram is ready.
375-394
: Similar consideration for error highlighting.
Likewise, the 220ms timeout may need to be replaced with a more robust sync or callback mechanism.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
api/src/chat/services/block.service.spec.ts
(3 hunks)api/src/chat/services/block.service.ts
(4 hunks)api/src/chat/services/bot.service.ts
(3 hunks)api/src/utils/test/mocks/conversation.ts
(3 hunks)api/src/websocket/websocket.gateway.ts
(2 hunks)api/types/event-emitter.d.ts
(1 hunks)frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
(4 hunks)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx
(5 hunks)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx
(3 hunks)frontend/src/styles/visual-editor.css
(4 hunks)frontend/src/websocket/socket-hooks.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
api/src/chat/services/block.service.spec.ts (1)
api/src/utils/test/mocks/conversation.ts (1)
subscriber
(38-40)
api/src/chat/services/bot.service.ts (1)
api/src/chat/schemas/conversation.schema.ts (1)
getDefaultConversationContext
(24-39)
api/src/chat/services/block.service.ts (2)
api/src/chat/schemas/types/message.ts (1)
StdOutgoingEnvelope
(325-325)api/src/chat/schemas/types/subscriberContext.ts (1)
SubscriberContext
(15-15)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (1)
NodeModel
(31-126)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (2)
BLOCK_WIDTH
(38-38)BLOCK_HEIGHT
(39-39)frontend/src/websocket/socket-hooks.tsx (1)
useSubscribe
(75-83)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: API-Tests
- GitHub Check: Frontend Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (46)
frontend/src/websocket/socket-hooks.tsx (1)
2-2
: Copyright year updated correctly.The copyright year has been updated to 2025, ensuring legal compliance.
api/src/utils/test/mocks/conversation.ts (3)
2-2
: Copyright year updated correctly.The copyright year has been updated to 2025, ensuring legal compliance.
11-11
: Appropriate import added for Subscriber type.The Subscriber import is correctly added to support the new subscriber mock object.
38-40
: Well-structured mock object for testing.The new
subscriber
constant enhances the test mocks by providing a properly structured Subscriber object. This will help maintain consistency across tests and support the new block highlighting functionality.The implementation aligns with the PR objective to support block highlighting features, making it easier to test the relevant functionality.
api/src/chat/services/block.service.spec.ts (3)
49-50
: Updated import to use the new subscriber mock.The import statement has been correctly updated to use the new
subscriber
constant instead ofsubscriberContextBlankInstance
.
432-433
: Test updated to use the new subscriber object.The test for processing list messages has been properly updated to use the new
subscriber
object, which maintains consistency with implementation changes in the service layer.
466-467
: Test updated to use the new subscriber object.The second test case for processing list messages with different parameters has also been properly updated to use the
subscriber
object.api/types/event-emitter.d.ts (1)
123-137
: Good implementation of type definitions for block highlighting events.The new
highlight
property added to theIHookOperationMap
interface provides well-structured type definitions for both normal block highlighting and error highlighting. This supports the PR's main objective to enhance the visual editor with highlighting features.The structure includes all necessary fields (userId, flowId, blockId) for both block and error scenarios, ensuring a consistent event structure between different highlighting operations. This will make implementing and maintaining the feature easier throughout the codebase.
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (5)
17-17
: Import added for listening to node state changes.This import is essential for implementing the state change listening mechanism. Good addition.
246-246
: Added property to store listener reference.Adding a property to store the listener reference follows best practices for React component state management.
258-262
: Implemented ComponentDidMount to register state change listener.Good implementation of the componentDidMount lifecycle method to register a listener on the node. This ensures the component re-renders when the node's state changes (highlighted or errored).
264-268
: Proper cleanup in ComponentWillUnmount.Excellent implementation of cleanup logic in componentWillUnmount. This prevents memory leaks by properly deregistering the listener when the component is unmounted.
272-277
: Consolidated styling logic.Good refactoring of the CSS class handling logic to include highlight and error states. The use of a variable makes the code more readable and maintainable.
api/src/chat/services/bot.service.ts (4)
139-142
: Changed processMessage parameter from context to recipient.This change aligns with the updated method signature in the BlockService. Now passing the entire recipient object instead of just the context.
143-147
: Added event emission for successful block highlighting.Excellent addition of event emission for highlighting triggered blocks. This event provides all necessary information: flowId, blockId, and userId, which will allow the frontend to highlight the correct block when a message flows through it.
315-321
: Added error highlighting event emission.Good implementation of error highlighting event emission. The conditional check ensures that errors are only reported for non-fallback blocks, preventing false positives.
509-516
: Improved fallback handling with proper recipient context.Good improvement to the global fallback handling. Creating a structured clone of the recipient and initializing the context variables ensures proper data flow.
frontend/src/styles/visual-editor.css (7)
37-38
: CSS formatting improvement.Minor formatting improvement for color value. Using lowercase for hex colors is a common CSS best practice.
186-186
: CSS transition formatting improvement.Minor formatting improvement for transition timing function. Using explicit decimal notation (0.4s vs .4s) improves readability.
197-198
: CSS scale formatting improvement.Improved readability by using explicit decimal notation (0.75 vs .75) for scale values in transform properties.
Also applies to: 201-202
215-223
: Added highlighted node styling.Excellent implementation of CSS styling for highlighted nodes. The styling includes elevation (z-index), scaling, and a pulsing animation effect to draw attention to the highlighted node.
225-242
: Created highlight pulse animation.Well-defined keyframe animation for highlighted nodes that creates a smooth, attention-grabbing blue pulse effect. The animation transitions properly between different scales and shadow intensities.
244-252
: Added error node styling.Great implementation of the error styling that makes errors visually distinct with red highlighting. This maintains consistency with the highlight styling while clearly indicating an error state.
254-271
: Created error pulse animation.Well-implemented keyframe animation for error states using red color to clearly indicate problems. The animation sequence matches the highlight animation for consistency.
api/src/chat/services/block.service.ts (5)
477-480
: Updated method signature to use recipient instead of subscriberContext.Good change to the processMessage method signature, replacing subscriberContext with the recipient object. This provides more context and flexibility to the method.
481-481
: Extract subscriberContext from recipient.Proper extraction of the subscriberContext from the recipient object, maintaining backward compatibility with the rest of the method.
571-586
: Added error event emission for deprecated attachment URLs.Good implementation of error reporting for deprecated attachment URLs. The code properly extracts the flowId and emits the error event with all necessary details.
635-650
: Added error event emission for content retrieval failures.Well-implemented error reporting for content retrieval failures. This ensures that blocks with content that cannot be loaded are properly highlighted as errors in the UI.
672-687
: Added error event emission for plugin processing failures.Excellent implementation of error reporting for plugin processing failures. This completes the error handling for all major failure points in the block processing pipeline.
api/src/websocket/websocket.gateway.ts (3)
9-13
: Ensure proper handling of event data.
Importing typed definitions from@nestjs/event-emitter
is beneficial. Verify that event data broadcast through these classes does not contain sensitive information or PII without proper safeguards.
411-422
: Review the “todo” and validate broadcast scope.
Currently,handleHighlightBlock
emits to all connected clients. The comment suggests restricting to a specific subscriber or flow. Confirm whether the payload includes private data and, if so, consider scoping the broadcast more narrowly.
424-431
: Confirm error broadcast scope and address “todo.”
Likewise,highlightBlockErrored
emits to all clients. Ensure users/subscribers that receive these error events are authorized and have a valid session.frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (5)
2-2
: License year update noted.
No further remarks.
12-12
: Alias usage is clear.
RenamingDefaultNodeModel
toStormNodeModel
helps distinguish it from the custom node.
27-28
: New interface properties for highlighting and error states.
The extendedNodeModelOptions
interface is appropriately typed for your new features.
44-46
: Defaults for highlight and error states.
Providing default values for_isHighlighted
and_hasErrored
ensures consistent initialization.
91-97
: Implementation for error state is consistent and clear.
The event firing insetHasErrored
is a good way to notify listeners.frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (9)
11-11
: Using Next.js router.
No issues integratingnext/router
.
16-16
: Expanded import to include RouterType.
No concerns; usage appears straightforward.
24-24
: WebSocket subscription utility.
TheuseSubscribe
hook is consistent with the rest of the codebase.
259-259
: Local router reference.
Implementing localrouter
usage for flow navigation is fine.
297-319
: Block visibility check.
The logic to compare transformed block coordinates to the canvas bounding box seems sound.
321-335
: Centering a block in the viewport.
This straightforward offset logic is correct.
337-350
: Redirect function looks good.
The sequence of setting the selected flow ID and routing is appropriate.
395-395
: Subscription to “highlight:flow.”
No concerns; hooking into the highlight flow event is straightforward.
397-397
: Subscription to “highlight:error.”
No concerns; the logic is consistent with flow-highlighting.
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
(4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (1)
NodeModel
(31-126)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (2)
BLOCK_WIDTH
(38-38)BLOCK_HEIGHT
(39-39)frontend/src/websocket/socket-hooks.tsx (1)
useSubscribe
(75-83)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Frontend Tests
- GitHub Check: API-Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (6)
11-11
: New imports and hooks added properly to support highlighting functionality.The added imports for
useRouter
, extendedEntityType
withRouterType
, anduseSubscribe
are correctly implemented to support the new highlighting and navigation features.Also applies to: 16-16, 24-24, 259-259
47-48
: Node model properties correctly initialized.The new properties
_hasErrored
and_isHighlighted
are properly initialized asfalse
in theNodeModel
constructor options, which aligns with the implementation in theNodeModel
class.
275-296
: Well-implemented highlight removal function.The
removeHighlights
function correctly:
- Returns a Promise for proper async handling
- Iterates through all nodes to reset both highlight and error states
- Repaints the canvas to reflect visual changes
- Properly resolves the Promise
297-319
: Block visibility check function is correctly implemented.The
isBlockVisibleOnCanvas
function properly calculates whether a block is within the visible area of the canvas, accurately accounting for zoom level, canvas offset, and block dimensions.
321-335
: Block centering function works correctly.The
centerBlockInView
function correctly calculates the offset needed to center a block in the canvas view and updates the canvas offset accordingly.
395-397
: Event subscriptions are properly implemented.The WebSocket event subscriptions for
highlight:block
andhighlight:error
are correctly implemented using theuseSubscribe
hook, which will properly handle component lifecycle for subscription cleanup.
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
Outdated
Show resolved
Hide resolved
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: 2
🧹 Nitpick comments (2)
api/src/websocket/websocket.gateway.ts (1)
412-425
: Event handlers for block highlighting feature.These event handlers properly implement the server-side logic for broadcasting block highlighting and error events to the appropriate WebSocket clients.
A few improvements could be made:
- Since these methods are marked as
async
but don't contain anyawait
operations, the async keyword could be removed- Consider adding error handling around the socket emission
- @OnEvent('hook:highlight:block') - async handleHighlightBlock( - payload: IHookOperationMap['highlight']['operations']['block'], - ) { - this.io.to(`blocks:${payload.userId}`).emit('highlight:block', payload); - } + @OnEvent('hook:highlight:block') + handleHighlightBlock( + payload: IHookOperationMap['highlight']['operations']['block'], + ) { + try { + this.io.to(`blocks:${payload.userId}`).emit('highlight:block', payload); + } catch (error) { + this.logger.error('Failed to emit highlight:block event', error); + } + } - @OnEvent('hook:highlight:error') - async highlightBlockErrored( - payload: IHookOperationMap['highlight']['operations']['error'], - ) { - this.logger.warn('hook:highlight:error ', payload); - this.io.to(`blocks:${payload.userId}`).emit('highlight:error', payload); - } + @OnEvent('hook:highlight:error') + highlightBlockErrored( + payload: IHookOperationMap['highlight']['operations']['error'], + ) { + this.logger.warn('hook:highlight:error ', payload); + try { + this.io.to(`blocks:${payload.userId}`).emit('highlight:error', payload); + } catch (error) { + this.logger.error('Failed to emit highlight:error event', error); + } + }api/src/chat/services/block.service.ts (1)
609-624
: Duplicated error handling logic.This error handling code properly emits events for the block highlighting feature, but the same pattern is repeated in three different places.
Consider refactoring this repeated error handling logic into a helper method to improve maintainability.
+ private emitHighlightError(block: Block | BlockFull, recipient: Subscriber) { + const flowId = typeof block.category === 'object' + ? block!.category.id + : block.category; + + if (flowId) { + this.logger.log('triggered: hook:highlight:error'); + this.eventEmitter.emit('hook:highlight:error', { + flowId, + userId: recipient.id, + blockId: block.id, + }); + } else { + this.logger.warn('Unable to trigger: hook:highlight:error'); + } + } - const flowId = - typeof block.category === 'object' - ? block!.category.id - : block.category; - if (flowId) { - this.logger.log('triggered: hook:highlight:error'); - this.eventEmitter.emit('hook:highlight:error', { - flowId, - userId: recipient.id, - blockId: block.id, - }); - } else { - this.logger.warn('Unable to trigger: hook:highlight:error'); - } + this.emitHighlightError(block, recipient);Apply this refactoring to the other two occurrences of this pattern at lines 673-688 and 710-725.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/src/chat/services/block.service.ts
(7 hunks)api/src/chat/services/bot.service.ts
(3 hunks)api/src/websocket/websocket.gateway.ts
(2 hunks)frontend/src/components/visual-editor/v2/Diagrams.tsx
(2 hunks)frontend/src/websocket/socket-hooks.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/websocket/socket-hooks.tsx
- api/src/chat/services/bot.service.ts
🧰 Additional context used
🧬 Code Definitions (2)
frontend/src/components/visual-editor/v2/Diagrams.tsx (1)
frontend/src/websocket/socket-hooks.tsx (1)
useSocketGetQuery
(87-105)
api/src/chat/services/block.service.ts (2)
api/src/chat/schemas/types/message.ts (1)
StdOutgoingEnvelope
(325-325)api/src/chat/schemas/types/subscriberContext.ts (1)
SubscriberContext
(15-15)
🪛 Biome (1.9.4)
api/src/chat/services/block.service.ts
[error] 82-82: This is an unexpected use of the debugger statement.
Unsafe fix: Remove debugger statement
(lint/suspicious/noDebugger)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: API-Tests
- GitHub Check: Frontend Tests
🔇 Additional comments (5)
frontend/src/components/visual-editor/v2/Diagrams.tsx (1)
55-55
: Import for new block highlighting feature.The import
useSocketGetQuery
is added to enable real-time block highlighting functionality, aligning with the PR objectives.api/src/websocket/websocket.gateway.ts (1)
9-13
: Import dependencies for event-driven block highlighting.The added imports from
@nestjs/event-emitter
enable the gateway to listen for and handle block highlighting events.api/src/chat/services/block.service.ts (3)
9-37
: Updated imports to support WebSocket functionality.Added imports for WebSocket-related functionality that enables block highlighting through real-time communication.
62-77
: Updated constructor to support WebSocket integration.The constructor now properly handles an optional WebSocket gateway dependency, enabling the service to emit events for block highlighting.
515-520
: Parameter renamed for improved clarity.The parameter has been renamed from
subscriberContext
torecipient
to better represent its purpose, with the subscriber context now derived from the recipient object.
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: 0
♻️ Duplicate comments (2)
api/src/chat/services/block.service.ts (2)
672-686
: Duplicate error handling logic.This is the second instance of the same error handling logic. Consider using the helper method suggested earlier to reduce code duplication.
709-723
: Duplicate error handling logic.This is the third instance of the same error handling logic. Consider using the helper method suggested earlier to reduce code duplication.
🧹 Nitpick comments (1)
api/src/chat/services/block.service.ts (1)
607-623
: Consider extracting the error highlighting logic to avoid duplication.This error handling and event emission logic is duplicated in three locations (here and at lines 672-686, 709-723). Consider extracting this into a helper method.
+ private emitBlockHighlightError(block: Block | BlockFull, recipient: Subscriber) { + const flowId = + typeof block.category === 'object' + ? // @ts-expect-error : block always has category + block!.category.id + : block.category; + if (flowId) { + this.logger.log('triggered: hook:highlight:error'); + this.eventEmitter.emit('hook:highlight:error', { + flowId, + userId: recipient.id, + blockId: block.id, + }); + } else { + this.logger.warn('Unable to trigger: hook:highlight:error'); + } + } // Then replace each occurrence with: - const flowId = - typeof block.category === 'object' - ? // @ts-expect-error : block always has category - block!.category.id - : block.category; - if (flowId) { - this.logger.log('triggered: hook:highlight:error'); - this.eventEmitter.emit('hook:highlight:error', { - flowId, - userId: recipient.id, - blockId: block.id, - }); - } else { - this.logger.warn('Unable to trigger: hook:highlight:error'); - } + this.emitBlockHighlightError(block, recipient);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/chat/services/block.service.ts
(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: API-Tests
- GitHub Check: Frontend Tests
🔇 Additional comments (4)
api/src/chat/services/block.service.ts (4)
9-36
: LGTM: WebSocket imports and dependencies are properly added.The required WebSocket-related imports and dependencies have been correctly added to support the new highlight blocks functionality.
62-77
: WebSocket gateway initialization looks good.The private gateway property and initialization in the constructor are well-implemented with proper null checking. The Optional decorator is correctly used to make the gateway a non-required dependency.
79-95
: WebSocket subscription endpoint is well-implemented.The socket subscription endpoint handles both GET and POST methods, joining users to their block-specific socket rooms. The error handling with proper logging is a good practice.
514-518
: Good parameter refactoring.Changing from
subscriberContext
torecipient
and then extractingsubscriberContext
fromrecipient.context
improves the API design and is consistent with the PR objective to highlight blocks.
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: 0
🧹 Nitpick comments (2)
api/src/websocket/websocket.gateway.ts (2)
412-419
: Well-implemented event handler for block highlighting.The
handleHighlightBlock
method correctly handles the 'hook:highlight:block' event and emits the appropriate socket event to the client. The logging statement is helpful for debugging purposes.Consider adding this room type to the
Room
enum for consistency with other room naming patterns in this file.// In the Room enum (line 44) + BLOCKS_USER = 'blocks',
@OnEvent('hook:highlight:block') async handleHighlightBlock( payload: IHookOperationMap['highlight']['operations']['block'], ) { this.logger.log('highlighting block', payload); - this.io.to(`blocks:${payload.userId}`).emit('highlight:block', payload); + this.io.to(`${Room.BLOCKS_USER}:${payload.userId}`).emit('highlight:block', payload); }
420-426
: Good implementation of error highlighting event handler.The
highlightBlockErrored
method appropriately handles the 'hook:highlight:error' event with proper warning level logging. This aligns with the PR objective of implementing error highlighting for plugins that encounter issues.Consider adding JSDoc comments to document the purpose and behavior of these methods, which would aid future maintainers. Also, apply the same room naming suggestion as above for consistency.
+ /** + * Handles events when a block should be highlighted with an error state + * @param payload The payload containing error and user information + */ @OnEvent('hook:highlight:error') async highlightBlockErrored( payload: IHookOperationMap['highlight']['operations']['error'], ) { this.logger.warn('hook:highlight:error', payload); - this.io.to(`blocks:${payload.userId}`).emit('highlight:error', payload); + this.io.to(`${Room.BLOCKS_USER}:${payload.userId}`).emit('highlight:error', payload); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/src/chat/services/block.service.ts
(7 hunks)api/src/websocket/websocket.gateway.ts
(2 hunks)frontend/src/websocket/socket-hooks.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/websocket/socket-hooks.tsx
- api/src/chat/services/block.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: API-Tests
- GitHub Check: Frontend Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
api/src/websocket/websocket.gateway.ts (1)
9-13
: Clean import structure for event handling.The additional import of
IHookOperationMap
from '@nestjs/event-emitter' is appropriate for the new event handlers being added. The import structure maintains consistency with the existing code style.
I'm noticed this error shows up in the API logs: This error occurs when handling a local fallback (user provides invalid input). It seems the block object passed to triggerBlock in this fallback scenario has block.category set to null. The code then tries to access block.category.id, leading to the error.
|
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: 2
🔭 Outside diff range comments (1)
api/src/chat/controllers/subscriber.controller.spec.ts (1)
1-237
: 💡 Verification agent🧩 Analysis chain
Verify null safety for category access in triggerBlock.
Based on PR comments, there's a reported TypeError: "Cannot read properties of null (reading 'id')" in the
triggerBlock
function when handling fallback scenarios withblock.category
set tonull
.
🏁 Script executed:
#!/bin/bash # Verify if there's null checking for block.category in the triggerBlock function echo "Searching for triggerBlock implementations..." rg -A 5 "triggerBlock.*\(" --type ts echo "Checking for null safety patterns around category.id access..." rg "category\?.id|category\s+&&\s+category\.id|category\s+!==\s+null" --type tsLength of output: 2708
Guard against null
category
in triggerBlock before accessingid
The call to
matchedBlock!.category!.id
intriggerBlock
can throw whencategory
is null. We didn’t find any existing null-safety checks around this access. Please update this location (and any similar emits) to handle a missing category, for example:• File:
api/src/chat/services/bot.service.ts
Around the event emit intriggerBlock
:// before this.eventEmitter.emit('hook:highlight:error', { flowId: matchedBlock!.category!.id, userId: convo.sender.id, }); // after this.eventEmitter.emit('hook:highlight:error', { - flowId: matchedBlock!.category!.id, + flowId: matchedBlock?.category?.id ?? fallbackFlowId, userId: convo.sender.id, });• Decide on a suitable fallback for
flowId
(e.g. skip the emit, use a default ID, or handle the error upstream).
• Apply the same check whereverblock.category.id
ormatchedBlock.category.id
is accessed.This will prevent the
TypeError: Cannot read properties of null (reading 'id')
.
🧹 Nitpick comments (1)
api/src/chat/services/block.service.ts (1)
1-2
: Update copyright year to current year.The copyright notice shows the year 2025, which appears to be a future date. Consider updating it to the current year (2024).
-/* Copyright © 2025 Hexastack. All rights reserved. +/* Copyright © 2024 Hexastack. All rights reserved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
api/src/chat/controllers/block.controller.spec.ts
(3 hunks)api/src/chat/controllers/subscriber.controller.spec.ts
(4 hunks)api/src/chat/services/block.service.spec.ts
(6 hunks)api/src/chat/services/block.service.ts
(7 hunks)api/src/setting/seeds/setting.seed-model.ts
(1 hunks)api/src/setting/services/setting.service.ts
(1 hunks)api/src/user/controllers/auth.controller.spec.ts
(3 hunks)api/src/websocket/websocket.gateway.spec.ts
(2 hunks)api/src/websocket/websocket.gateway.ts
(4 hunks)frontend/public/locales/en/chatbot_settings.json
(1 hunks)frontend/public/locales/fr/chatbot_settings.json
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- api/src/chat/controllers/block.controller.spec.ts
- frontend/public/locales/en/chatbot_settings.json
- api/src/user/controllers/auth.controller.spec.ts
- frontend/public/locales/fr/chatbot_settings.json
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/chat/services/block.service.spec.ts
- api/src/websocket/websocket.gateway.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Frontend Tests
- GitHub Check: API-Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
api/src/setting/services/setting.service.ts (1)
168-171
: Well-implementedisHighlightEnabled
method.The method correctly retrieves settings and provides a default value of
false
if the debug setting is not defined. This is a good practice for feature toggles.api/src/setting/seeds/setting.seed-model.ts (1)
88-94
: Appropriate addition of the debugging toggle.The new
enable_debug
setting is well-structured: placed in the correct group, uses the appropriate checkbox type, and has a sensible default offalse
. Setting it disabled by default is a good practice for debugging features.api/src/websocket/websocket.gateway.spec.ts (2)
9-21
: Necessary imports for testing highlight functionality.The additions of the cache, event emitter, logger, and setting module imports are appropriate for supporting testing of the new highlight features.
38-75
: Complete test environment configuration.The test module setup has been properly extended with all necessary providers and imports. The event emitter configuration, cache module, and setting service dependencies are essential for testing the new highlight functionality.
api/src/chat/controllers/subscriber.controller.spec.ts (3)
9-21
: Appropriate imports for test module setup.These imports support the testing of functionality that may interact with the highlighting feature.
68-101
: Well-configured test module imports.The test setup has been properly enhanced with cache support, event emitter, logger, and settings module. This ensures all necessary dependencies for the highlighting feature are available during testing.
117-127
: Proper mock providers for testing.The addition of the setting-related services and mock cache manager ensures proper testing of functionality that depends on these services.
api/src/chat/services/block.service.ts (3)
82-115
: Well-implemented WebSocket subscription endpoint.The subscription mechanism is properly implemented with appropriate error handling and logging. The method correctly extracts the subscriber ID from the session, finds the corresponding subscriber, and joins the socket room.
534-538
: Good refactor: renamed parameter for improved clarity.Changing from
subscriberContext
torecipient
with aSubscriber
type provides better clarity and type safety, while deriving the context from the recipient object.
89-104
:❌ Incorrect review comment
Handle potential null/undefined block category in fallback scenarios.
The PR objectives mention a TypeError when accessing
block.category.id
in fallback scenarios wherecategory
could be null. While the current implementation adds a check, consider making this more robust throughout the code.
🏁 Script executed:
#!/bin/bash # Search for places where block.category is accessed without null checks echo "Searching for potential null access on block.category..." rg -n "block\\.category\\." --no-heading api/src/chat/ # Also check for places where the triggerBlock function is called echo "Searching for triggerBlock function calls..." rg -n "triggerBlock" --no-heading api/src/Length of output: 950
Ignore null/undefined
block.category
suggestion
A search acrossapi/src/chat
revealed no occurrences ofblock.category
(and thus no unguardedblock.category.id
accesses). There are no fallback code paths needing additional null checks forblock.category
.Likely an incorrect or invalid review comment.
6138006
to
6ee59d4
Compare
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
♻️ Duplicate comments (2)
api/src/chat/services/block.service.ts (2)
628-643
: 🛠️ Refactor suggestionExtract duplicated error emission logic to a helper method.
This error emission logic is duplicated in three places (lines 628-643, 692-707, and 729-744), violating the DRY principle.
Create a reusable helper method:
+ /** + * Emits a block error highlight event if the block has a valid category + * + * @param block - The block that encountered an error + * @param recipientId - The ID of the recipient + */ + private emitBlockError(block: Block | BlockFull, recipientId: string): void { + const flowId = + typeof block.category === 'object' + ? block.category?.id + : block.category; + + if (flowId) { + this.logger.log('triggered: hook:highlight:error'); + this.eventEmitter.emit('hook:highlight:error', { + flowId, + userId: recipientId, + blockId: block.id, + }); + } else { + this.logger.warn('Unable to trigger: hook:highlight:error'); + } + }Then use it in all three places:
- const flowId = - typeof block.category === 'object' - ? // @ts-expect-error : block always has category - block!.category.id - : block.category; - if (flowId) { - this.logger.log('triggered: hook:highlight:error'); - this.eventEmitter.emit('hook:highlight:error', { - flowId, - userId: recipient.id, - blockId: block.id, - }); - } else { - this.logger.warn('Unable to trigger: hook:highlight:error'); - } + this.emitBlockError(block, recipient.id);
629-632
:⚠️ Potential issueFix type handling instead of using @ts-expect-error.
Using
@ts-expect-error
and non-null assertion (!
) is error-prone and masks potential issues. The comment "block always has category" may not be true in all cases, as shown by the null check in the subsequent lines.Use optional chaining to safely handle null categories:
const flowId = typeof block.category === 'object' - ? // @ts-expect-error : block always has category - block!.category.id + ? block.category?.id : block.category;This change will prevent the TypeError mentioned in PR feedback: "Cannot read properties of null (reading 'id')".
🧹 Nitpick comments (1)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (1)
352-379
: Refactor duplicate highlight handler logic.The
handleHighlightFlow
andhandleHighlightErroredBlock
functions contain significant code duplication. Consider refactoring to a shared utility function.+async function highlightBlock(payload: any, isError = false) { + await removeHighlights(); + await redirectToTriggeredFlow(selectedCategoryId, payload.flowId); + + return new Promise<void>((resolve) => { + setTimeout(() => { + const block = engine?.getModel().getNode(payload.blockId) as NodeModel | undefined; + + if (!block) { + resolve(); + return; + } + + if (!isBlockVisibleOnCanvas(block)) { + centerBlockInView(block); + } + + if (isError) { + block.setHasErrored(true); + } else { + block.setSelected(true); + block.setHighlighted(true); + } + + engine.repaintCanvas(); + resolve(); + }, 200); + }); +} + +async function handleHighlightFlow(payload: { + flowId: string; + blockId: string; + userId: string; +}) { + await highlightBlock(payload); +} + +async function handleHighlightErroredBlock(payload: { + flowId: string; + blockId: string; + userId: string; +}) { + await highlightBlock(payload, true); +}Also applies to: 381-406
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
api/src/chat/controllers/block.controller.spec.ts
(3 hunks)api/src/chat/controllers/subscriber.controller.spec.ts
(4 hunks)api/src/chat/services/block.service.spec.ts
(6 hunks)api/src/chat/services/block.service.ts
(7 hunks)api/src/chat/services/bot.service.ts
(3 hunks)api/src/setting/seeds/setting.seed-model.ts
(1 hunks)api/src/setting/services/setting.service.ts
(1 hunks)api/src/user/controllers/auth.controller.spec.ts
(3 hunks)api/src/utils/test/mocks/conversation.ts
(3 hunks)api/src/websocket/websocket.gateway.spec.ts
(2 hunks)api/src/websocket/websocket.gateway.ts
(4 hunks)api/types/event-emitter.d.ts
(1 hunks)frontend/public/locales/en/chatbot_settings.json
(1 hunks)frontend/public/locales/fr/chatbot_settings.json
(1 hunks)frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
(4 hunks)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx
(5 hunks)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx
(3 hunks)frontend/src/components/visual-editor/v2/Diagrams.tsx
(2 hunks)frontend/src/styles/visual-editor.css
(4 hunks)frontend/src/websocket/socket-hooks.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
- frontend/public/locales/en/chatbot_settings.json
- frontend/src/websocket/socket-hooks.tsx
- api/src/utils/test/mocks/conversation.ts
- api/src/chat/services/block.service.spec.ts
- frontend/src/components/visual-editor/v2/Diagrams.tsx
- api/src/setting/services/setting.service.ts
- api/src/setting/seeds/setting.seed-model.ts
- api/src/websocket/websocket.gateway.spec.ts
- api/src/user/controllers/auth.controller.spec.ts
- frontend/public/locales/fr/chatbot_settings.json
- api/src/chat/controllers/block.controller.spec.ts
- api/src/chat/services/bot.service.ts
- api/src/websocket/websocket.gateway.ts
- frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx
- api/src/chat/controllers/subscriber.controller.spec.ts
- frontend/src/styles/visual-editor.css
- api/types/event-emitter.d.ts
- frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (1)
NodeModel
(31-126)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (2)
BLOCK_WIDTH
(38-38)BLOCK_HEIGHT
(39-39)frontend/src/websocket/socket-hooks.tsx (1)
useSubscribe
(74-82)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Frontend Tests
- GitHub Check: API-Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
api/src/chat/services/block.service.ts (4)
9-36
: Dependencies are well organized with proper importing strategy.The imports are logically grouped and organized. Good job adding the necessary WebSocket-related imports for the new functionality.
64-80
: Good use of dependency injection with Optional decorator.Using
@Optional()
for the gateway parameter is the right approach, making the code more testable while maintaining backward compatibility.
82-115
: Well-structured WebSocket subscription method.The subscription method properly handles authentication checking, subscriber lookup, room joining, and error cases. The error handling is comprehensive with appropriate logging.
531-539
: API signature change improves data access.Changing the method signature from accepting
subscriberContext
torecipient
of typeSubscriber
is a good design choice. This provides access to the full subscriber entity while still allowing access to the context.frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (6)
47-49
: Good initialization of node state properties.Adding
_hasErrored
and_isHighlighted
properties with default values ensures consistent state initialization. These properties align well with the NodeModel class enhancements.
275-296
: Well-implemented highlight removal function.The
removeHighlights
function properly clears all highlighted and errored states from nodes and repaints the canvas. Returning a Promise allows for proper sequencing of operations.
297-319
: Good implementation of viewport visibility check.The
isBlockVisibleOnCanvas
function correctly calculates whether a block is visible in the current viewport, taking into account zoom level, canvas dimensions, and scroll position.
321-335
: Well-implemented block centering function.The
centerBlockInView
function properly calculates the offset needed to center a block in the viewport, considering zoom level and block dimensions.
337-350
: Flow redirection logic is clean and efficient.The flow redirection function handles both the state update and navigation appropriately. It properly checks if a redirect is needed to avoid unnecessary navigation.
407-409
: Good use of WebSocket subscription hooks.The WebSocket event subscriptions are properly implemented using the custom hook. The event names match the backend emitted events, creating a cohesive real-time experience.
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
Outdated
Show resolved
Hide resolved
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
🔭 Outside diff range comments (1)
api/src/chat/services/block.service.ts (1)
726-750
:⚠️ Potential issueHandle potential null category in error highlighting.
According to the PR comments, there's a TypeError that occurs when trying to access
block.category.id
butcategory
is null. This needs to be addressed.The PR comments mention a TypeError: "Cannot read properties of null (reading 'id')" in the
triggerBlock
function. This likely relates to the error emission code where you're accessingblock.category.id
whencategory
is null. Ensure you handle this case:private emitBlockHighlight( block: Block | BlockFull, userId: string, highlightType: 'highlight' | 'error' | 'fallback' ): void { - const flowId = - typeof block.category === 'object' - ? block.category?.id - : block.category; + let flowId; + if (!block.category) { + this.logger.warn('Block has null category, cannot determine flow ID'); + return; + } + + flowId = typeof block.category === 'object' + ? block.category?.id + : block.category; if (flowId) { this.logger.log(`triggered: hook:highlight:${highlightType}`); this.eventEmitter.emit('hook:highlight:block', { userId, blockId: block.id, highlightType, }); } else { this.logger.warn(`Unable to trigger: hook:highlight:${highlightType}`); } }
♻️ Duplicate comments (2)
api/src/chat/services/block.service.ts (2)
682-688
: Same emission logic duplication.This is another instance of the duplicated error emission logic.
710-716
: Another instance of duplicated emission logic.This is the third occurrence of the duplicated error emission logic.
🧹 Nitpick comments (4)
api/src/websocket/websocket.module.ts (1)
2-2
: Minor update to copyright year.The copyright year has been updated to 2025, which is consistent with other files but exceeds the current year (2024).
-Copyright © 2025 Hexastack. All rights reserved. +Copyright © 2024 Hexastack. All rights reserved.frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (2)
356-358
: Consider using requestAnimationFrame instead of setTimeout.Using setTimeout for timing in UI operations can be unreliable. For better synchronization with rendering cycles, consider using requestAnimationFrame.
function wait(ms: number) { - return new Promise((resolve) => setTimeout(resolve, ms)); + return new Promise((resolve) => { + const start = performance.now(); + function checkTime() { + if (performance.now() - start >= ms) { + resolve(true); + } else { + requestAnimationFrame(checkTime); + } + } + requestAnimationFrame(checkTime); + }); }
360-394
: Refactor highlight handling to reduce code duplication.The highlight handling logic switches between three different highlight types but follows the same flow for each. This could be simplified to reduce code duplication.
async function handleHighlightFlow(payload: { flowId: string; blockId: string; highlightType: "highlight" | "error" | "fallback"; }) { await redirectToTriggeredFlow(selectedCategoryId, payload.flowId); await clearHighlights(); const block = engine?.getModel().getNode(payload.blockId) as | NodeModel | undefined; if (!block) { return; } if (!isBlockVisibleOnCanvas(block)) { centerBlockInView(block); } - switch (payload.highlightType) { - case "highlight": - block.setHighlighted(true); - break; - case "error": - block.setHasErrored(true); - break; - case "fallback": - block.setHasFallbacked(true); - break; - } + // Set the appropriate highlight state based on type + const highlightMethods = { + highlight: 'setHighlighted', + error: 'setHasErrored', + fallback: 'setHasFallbacked' + }; + const method = highlightMethods[payload.highlightType]; + block[method](true); block.setSelected(true); }api/src/chat/services/block.service.ts (1)
82-115
: Well-implemented WebSocket subscription endpoint.The subscribe method is properly implemented with socket decorators and includes good error handling. However, room naming could be more consistent.
Consider using a constant for the room name prefix to ensure consistency:
+ private readonly BLOCKS_ROOM_PREFIX = 'blocks:'; + @SocketGet('/block/subscribe/') @SocketPost('/block/subscribe/') async subscribe( @SocketReq() req: SocketRequest, @SocketRes() res: SocketResponse, ) { try { const subscriberForeignId = req.session.passport?.user?.id; if (!subscriberForeignId) { this.logger.warn('Missing subscriber foreign ID in session'); throw new Error('Invalid session or user not authenticated'); } const subscriber = await this.subscriberService.findOne({ foreign_id: subscriberForeignId, }); if (!subscriber) { this.logger.warn( `Subscriber not found for foreign ID ${subscriberForeignId}`, ); throw new Error('Subscriber not found'); } - const room = `blocks:${subscriber.id}`; + const room = `${this.BLOCKS_ROOM_PREFIX}${subscriber.id}`; this.gateway.io.socketsJoin(room); this.logger.log('Subscribed to socket room', room); return res.status(200).json({ success: true, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/src/chat/services/block.service.ts
(7 hunks)api/src/chat/services/bot.service.ts
(3 hunks)api/src/websocket/websocket.gateway.spec.ts
(2 hunks)api/src/websocket/websocket.gateway.ts
(5 hunks)api/src/websocket/websocket.module.ts
(1 hunks)api/types/event-emitter.d.ts
(2 hunks)frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
(4 hunks)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx
(5 hunks)frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx
(3 hunks)frontend/src/styles/visual-editor.css
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- api/src/chat/services/bot.service.ts
- frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx
- api/types/event-emitter.d.ts
- api/src/websocket/websocket.gateway.ts
- frontend/src/styles/visual-editor.css
- frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
api/src/websocket/websocket.module.ts (1)
api/src/chat/schemas/block.schema.ts (1)
BlockModel
(186-189)
api/src/websocket/websocket.gateway.spec.ts (1)
api/src/chat/schemas/block.schema.ts (1)
BlockModel
(186-189)
🔇 Additional comments (12)
api/src/websocket/websocket.module.ts (2)
10-17
: Good modular imports for block highlighting functionality.These imports provide the necessary components for the new block highlighting feature. The module structure follows good NestJS practices for feature organization.
24-32
: Well-structured module configuration with proper dependency registration.The module now correctly registers the BlockModel and SettingModel with Mongoose, and provides all necessary services and repositories for the highlight block feature. This follows the NestJS best practices for module composition.
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (5)
47-49
: Good addition of block state properties.These new properties will track different visual states for nodes (highlighted, errored, or fallbacked), which aligns with the PR objective to highlight triggered and errored blocks.
276-299
: Well-implemented highlight clearing function.The
clearHighlights
function properly resets all highlight states on nodes, checking each possible state. This is essential for ensuring that only one block is highlighted at a time.
300-322
: Good implementation of visibility detection.The
isBlockVisibleOnCanvas
function correctly calculates whether a block is within the viewport by considering zoom level, offsets, and canvas dimensions.
324-338
: Well-implemented block centering logic.The
centerBlockInView
function properly calculates the offsets needed to center a block in the canvas viewport, taking into account zoom level and canvas dimensions.
340-354
: Good implementation of flow navigation.The
redirectToTriggeredFlow
function correctly handles changing flows when a highlighted block is in a different flow than the currently visible one.api/src/websocket/websocket.gateway.spec.ts (3)
9-22
: Good comprehensive test setup with all required dependencies.The imports have been properly updated to include all the necessary components for testing the WebsocketGateway with the new block highlighting functionality.
42-46
: Complete provider registration for testing.All the necessary services and repositories have been registered as providers, ensuring proper dependency injection during tests.
49-78
: Well-configured test modules with appropriate settings.The test setup includes well-configured EventEmitterModule and CacheModule with detailed options that match production requirements. The MongooseModule is also properly set up to use test models.
api/src/chat/services/block.service.ts (2)
64-80
: Use Optional decorator for WebsocketGateway dependency.Good use of the @optional decorator to handle cases where WebsocketGateway might not be provided. This improves the service's reusability in different contexts.
538-538
: Good parameter refactoring.Replacing
subscriberContext
withrecipient
and deriving the context from it is a good refactoring that makes the code more maintainable.
Motivation
Screencast.from.07-04-25.13.03.35.webm
Before testing you might need to do
hexabot stop hexabot dev
This PR introduces the feature of highlighting trigger blocks in the visual editor. Additionally, it adds error highlighting for plugins when they encounter issues, improving the visibility and debugging experience for developers.
Fixes #884
Type of change:
Please delete options that are not relevant.
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style
Chores