Conversation
Update CHANGELOG.md for v2.5.3
Features/actions
update pipelines
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive Actions feature (v2.6.0) that enables automated modification of requests and responses in the Beo Echo API mocking service. The implementation includes:
- Two action types: Replace Text and Run JavaScript
- Full CRUD operations with priority-based execution ordering
- Filter system for conditional action execution
- Integration with existing mock/proxy service pipeline
- Complete frontend UI with drag-and-drop reordering
Key Changes
- Backend: New actions module with service layer, repository, and two action types (replace_text, run_javascript)
- Frontend: Complete Actions UI with wizards, forms, and interactive components
- Infrastructure: Database migrations, middleware updates, and request ID tracking
Reviewed Changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
backend/src/actions/ |
Complete actions feature implementation with service, handler, and modules |
backend/src/database/models.go |
Added Action and ActionFilter models |
backend/src/database/repositories/action_repo.go |
Repository implementation with tests |
backend/src/echo/services/mock_service.go |
Integration of action execution in request pipeline |
backend/src/middlewares/logRequestId.go |
New middleware for request ID tracking |
frontend/src/lib/components/actions/ |
Complete Actions UI components |
frontend/src/lib/api/actionsApi.ts |
Frontend API client for actions |
frontend/src/lib/types/Action.ts |
TypeScript type definitions |
docs/Actions_Documentation.md |
Comprehensive user guide |
| request_body: { | ||
| label: 'Response Body', | ||
| icon: 'fa-arrow-left', | ||
| color: 'text-green-500 dark:text-green-400' | ||
| }, |
There was a problem hiding this comment.
The label for 'request_body' is incorrectly set to 'Response Body' - it should be 'Request Body'. This mislabeling will confuse users about which target they are actually modifying.
|
|
||
| // ValidateRunJavascriptConfig validates the configuration for run_javascript actions | ||
| func (m *ModulesAction) ValidateRunJavascriptConfig(configJSON string) error { | ||
| // TODO : Implement actual validation logic if needed |
There was a problem hiding this comment.
The ValidateRunJavascriptConfig function is not implemented and always returns nil. This leaves JavaScript configurations unvalidated, potentially allowing invalid or malicious scripts through. At minimum, validate that the config is valid JSON and contains a non-empty 'script' field.
| // TODO : Implement actual validation logic if needed | |
| var cfg RunJavascriptConfig | |
| if err := json.Unmarshal([]byte(configJSON), &cfg); err != nil { | |
| return errors.New("invalid JSON for run_javascript config: " + err.Error()) | |
| } | |
| if strings.TrimSpace(cfg.Script) == "" { | |
| return errors.New("run_javascript config must contain a non-empty 'script' field") | |
| } |
| class="text-red-500 hover:text-red-400 text-sm" | ||
| on:click={() => removeFilter(index)} | ||
| title="Remove filter" | ||
| aria-label="Remove filter {index + 1}" |
There was a problem hiding this comment.
The icon-only button should include 'aria-label' without the {index + 1} template syntax inside the attribute value. This creates confusing output. Change to: aria-label="Remove filter" and let the context determine which filter is being removed, or properly interpolate: aria-label='Remove filter {index + 1}' (note single quotes for proper Svelte interpolation).
| aria-label="Remove filter {index + 1}" | |
| aria-label='Remove filter {index + 1}' |
| }) | ||
| vm.Set("console", consoleObj) | ||
|
|
||
| // Clone context to avoid modifying the original |
There was a problem hiding this comment.
The comment says 'Clone context' but the code is actually marshaling (serializing) the context to JSON, not cloning it. Update comment to: 'Serialize context to JSON for VM execution' to accurately describe the operation.
| // Clone context to avoid modifying the original | |
| // Serialize context to JSON for VM execution |
| // UpdateActionPriorityRequest represents the request to update action priority | ||
| // Priority is 1-based (starts from 1, not 0) |
There was a problem hiding this comment.
The comment correctly explains that priority is 1-based, but it would be helpful to also document the upper bound and what happens when the priority exceeds the number of actions in the execution point group. Add a comment explaining that priority values are constrained to the valid range [1, N] where N is the count of actions in the same execution point.
| // UpdateActionPriorityRequest represents the request to update action priority | |
| // Priority is 1-based (starts from 1, not 0) | |
| // UpdateActionPriorityRequest represents the request to update action priority. | |
| // Priority is 1-based (starts from 1, not 0). | |
| // Priority values are constrained to the valid range [1, N], where N is the count of actions in the same execution point group. | |
| // If the priority exceeds N, the request will be rejected with an error. |
| <div | ||
| bind:this={actionElements[action.id]} | ||
| role="button" | ||
| tabindex="0" | ||
| draggable="true" | ||
| on:dragstart={() => handleDragStart(action, idx)} | ||
| on:dragover={handleDragOver} | ||
| on:drop={(e) => handleDrop(e, action, idx)} | ||
| on:dragend={handleDragEnd} | ||
| on:keydown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
The keydown handler prevents default behavior for Enter and Space keys but doesn't actually perform any action. This breaks keyboard accessibility for users who can't use drag-and-drop. Implement keyboard-based reordering (e.g., using arrow keys or providing alternative action buttons) to ensure keyboard users can reorder actions.
| // Reorder priorities - only update affected actions in the same execution point | ||
| if oldPriority < newPriority { | ||
| // Moving down: shift actions up (decrease priority) | ||
| for i := range sameExecutionPointActions { | ||
| if sameExecutionPointActions[i].ID == actionID { | ||
| sameExecutionPointActions[i].Priority = newPriority | ||
| affectedActions = append(affectedActions, &sameExecutionPointActions[i]) | ||
| affectedCount++ | ||
| } else if sameExecutionPointActions[i].Priority > oldPriority && sameExecutionPointActions[i].Priority <= newPriority { | ||
| sameExecutionPointActions[i].Priority-- | ||
| affectedActions = append(affectedActions, &sameExecutionPointActions[i]) | ||
| affectedCount++ | ||
| } | ||
| } | ||
| } else { |
There was a problem hiding this comment.
The priority reordering logic modifies a slice element directly and then takes its address to append to affectedActions. This creates a pointer to a loop variable which could lead to subtle bugs. Consider refactoring to work with pointers throughout or create a separate variable for the modified action before appending.
| #### JavaScript Limitations: | ||
| - **Timeout:** 5 seconds maximum | ||
| - **No async/await** - Synchronous code only | ||
| - **No external libraries** - Standard JavaScript only (ES5) |
There was a problem hiding this comment.
The documentation states 'Standard JavaScript only (ES5)' but the implementation uses goja which supports ES5.1+ and many ES6 features. Update the documentation to accurately reflect the supported JavaScript version to avoid confusing users about what language features they can use.
| - **No external libraries** - Standard JavaScript only (ES5) | |
| - **No external libraries** - Standard JavaScript (ES5.1 and many ES6 features supported, e.g. `let`, `const`, arrow functions, etc.) |
No description provided.