Feat : Highlight triggered and errored blocks #896
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 and error states, along with corresponding CSS animations and styling updates. Minor adjustments such as variable renaming 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}
EE->>WG: Deliver hook:highlight:block event
WG->>WG: Invoke handleHighlightBlock
WG->>FE: Emit "highlight:flow" via socket with payload
FE->>FE: handleHighlightFlow -> Center block & update highlight state
Highlight Error 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:error {userId, flowId, blockId}
EE->>WG: Deliver hook:highlight:error event
WG->>WG: Invoke highlightBlockErrored
WG->>FE: Emit "highlight:error" via socket with payload
FE->>FE: handleHighlightErroredBlock -> Center block & update error state
Assessment against linked issues
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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,setHighlighteddoes 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.
SinceremoveHighlightsis 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
subscriberconstant 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
subscriberconstant 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
subscriberobject, 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
subscriberobject.api/types/event-emitter.d.ts (1)
123-137: Good implementation of type definitions for block highlighting events.The new
highlightproperty added to theIHookOperationMapinterface 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-emitteris 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,handleHighlightBlockemits 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,highlightBlockErroredemits 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.
RenamingDefaultNodeModeltoStormNodeModelhelps distinguish it from the custom node.
27-28: New interface properties for highlighting and error states.
The extendedNodeModelOptionsinterface is appropriately typed for your new features.
44-46: Defaults for highlight and error states.
Providing default values for_isHighlightedand_hasErroredensures consistent initialization.
91-97: Implementation for error state is consistent and clear.
The event firing insetHasErroredis 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.
TheuseSubscribehook is consistent with the rest of the codebase.
259-259: Local router reference.
Implementing localrouterusage 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.
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, extendedEntityTypewithRouterType, anduseSubscribeare 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
_hasErroredand_isHighlightedare properly initialized asfalsein theNodeModelconstructor options, which aligns with the implementation in theNodeModelclass.
275-296: Well-implemented highlight removal function.The
removeHighlightsfunction 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
isBlockVisibleOnCanvasfunction 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
centerBlockInViewfunction 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:blockandhighlight:errorare correctly implemented using theuseSubscribehook, which will properly handle component lifecycle for subscription cleanup.
| async function handleHighlightErroredBlock(payload: any) { | ||
| await removeHighlights(); | ||
|
|
||
| await redirectToTriggeredFlow(selectedCategoryId, payload.flowId); | ||
| setTimeout(() => { | ||
| const block = engine?.getModel().getNode(payload.blockId) as NodeModel; | ||
|
|
||
| if (!block) { | ||
| return; | ||
| } | ||
|
|
||
| if (!isBlockVisibleOnCanvas(block)) { | ||
| centerBlockInView(block); | ||
| } | ||
|
|
||
| block.setHasErrored(true); | ||
|
|
||
| engine.repaintCanvas(); | ||
| }, 220); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harmonize timeout values and consider refactoring.
There's an inconsistency between timeout values (200ms vs 220ms) in the highlight and error handling functions, which suggests they were chosen arbitrarily. Also, these functions share similar code structure.
Consider:
- Using the same timeout value for consistency
- Extracting the common logic into a shared function:
function highlightBlock(payload: any, isError: boolean) {
return new Promise<void>(async (resolve) => {
await removeHighlights();
await redirectToTriggeredFlow(selectedCategoryId, payload.flowId);
requestAnimationFrame(() => {
const block = engine?.getModel().getNode(payload.blockId) as NodeModel;
if (!block) {
resolve();
return;
}
if (!isBlockVisibleOnCanvas(block)) {
centerBlockInView(block);
}
if (isError) {
block.setHasErrored(true);
} else {
block.setSelected(true);
block.setHighlighted(true);
}
engine.repaintCanvas();
resolve();
});
});
}
// Then use it like:
async function handleHighlightFlow(payload: any) {
await highlightBlock(payload, false);
}
async function handleHighlightErroredBlock(payload: any) {
await highlightBlock(payload, true);
}
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx
Outdated
Show resolved
Hide resolved
| async function handleHighlightFlow(payload: any) { | ||
| await removeHighlights(); | ||
|
|
||
| await redirectToTriggeredFlow(selectedCategoryId, payload.flowId); | ||
|
|
||
| setTimeout(() => { | ||
| const block = engine?.getModel().getNode(payload.blockId) as NodeModel; | ||
|
|
||
| if (!block) { | ||
| return; | ||
| } | ||
|
|
||
| if (!isBlockVisibleOnCanvas(block)) { | ||
| centerBlockInView(block); | ||
| } | ||
|
|
||
| block.setSelected(true); | ||
| block.setHighlighted(true); | ||
|
|
||
| engine.repaintCanvas(); | ||
| }, 200); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider an alternative to setTimeout.
While the handleHighlightFlow function correctly highlights a block and centers it in the view, using setTimeout for synchronization can be unreliable, especially with variable network or rendering delays.
Consider using an event-based approach or a more reliable method to detect when the canvas is ready for interaction. For example:
- setTimeout(() => {
- const block = engine?.getModel().getNode(payload.blockId) as NodeModel;
- // rest of the function
- }, 200);
+ // Option 1: Use requestAnimationFrame for better synchronization with rendering
+ requestAnimationFrame(() => {
+ const block = engine?.getModel().getNode(payload.blockId) as NodeModel;
+ if (!block) return;
+
+ // If block still not found, it might need more time to render
+ if (!isBlockVisibleOnCanvas(block) && !block) {
+ // Try again in the next frame
+ requestAnimationFrame(() => {
+ const retryBlock = engine?.getModel().getNode(payload.blockId) as NodeModel;
+ if (!retryBlock) return;
+ centerAndHighlightBlock(retryBlock);
+ });
+ return;
+ }
+
+ centerAndHighlightBlock(block);
+ });
+
+ function centerAndHighlightBlock(block: NodeModel) {
+ if (!isBlockVisibleOnCanvas(block)) {
+ centerBlockInView(block);
+ }
+ block.setSelected(true);
+ block.setHighlighted(true);
+ engine.repaintCanvas();
+ }Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
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
asyncbut don't contain anyawaitoperations, 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
useSocketGetQueryis 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-emitterenable 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
subscriberContexttorecipientto better represent its purpose, with the subscriber context now derived from the recipient object.
There was a problem hiding this comment.
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
subscriberContexttorecipientand then extractingsubscriberContextfromrecipient.contextimproves the API design and is consistent with the PR objective to highlight blocks.
abdou6666
requested review from
marrouchi,
medchedli,
IkbelTalebHssan,
yassinedorbozgithub and
MohamedAliBouhaouala
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
api/src/websocket/websocket.gateway.ts (2)
412-419: Well-implemented event handler for block highlighting.The
handleHighlightBlockmethod 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
Roomenum 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
highlightBlockErroredmethod 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
IHookOperationMapfrom '@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. |
Motivation
Screencast.from.07-04-25.13.03.35.webm
Before testing you might need to do
hexabot stop hexabot devThis 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
New Features
Style
Chores