feat: edit robot target url #485
Conversation
WalkthroughThis pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal as RobotEditModal
participant API
participant Server
User->>Modal: Enter target URL in TextField
Modal->>Modal: handleTargetUrlChange() updates robot state
User->>Modal: Click Save
Modal->>API: Call updateRecording({ targetUrl, ... })
API->>Server: PUT /recordings/:id with targetUrl
Server->>Server: Validate input & update workflow steps (replace URLs)
Server-->>API: Return updated recording
API-->>Modal: Send response back to UI
Possibly related PRs
Suggested labels
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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:
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: 0
🧹 Nitpick comments (6)
server/src/routes/storage.ts (3)
265-266: Update error message to match parameter name.There's a discrepancy between the error message which refers to "target_url" and the actual parameter name "targetUrl".
- return res.status(400).json({ error: 'Either "name", "limit" or "target_url" must be provided.' }); + return res.status(400).json({ error: 'Either "name", "limit" or "targetUrl" must be provided.' });
281-297: Consider adding URL validation.The code updates the workflow with the provided target URL without any validation. Consider adding basic URL validation to prevent potential issues.
if (targetUrl) { + // Basic URL validation + try { + new URL(targetUrl); + } catch (error) { + return res.status(400).json({ error: 'Invalid URL format.' }); + } const updatedWorkflow = robot.recording.workflow.map((step) => { if (step.where?.url && step.where.url !== "about:blank") { step.where.url = targetUrl; } step.what.forEach((action) => { if (action.action === "goto" && action.args?.length) { action.args[0] = targetUrl; } }); return step; });
281-297: Consider refactoring duplicated URL update logic.The URL update logic is duplicated in the
duplicateendpoint (lines 399-411). Consider extracting this logic into a reusable helper function.// Suggested helper function to place at the module level function updateWorkflowUrls(workflow: any[], targetUrl: string): any[] { return workflow.map((step) => { if (step.where?.url && step.where.url !== "about:blank") { step.where.url = targetUrl; } step.what.forEach((action) => { if (action.action === "goto" && action.args?.length) { action.args[0] = targetUrl; } }); return step; }); }Then use it in both places:
// In the PUT endpoint const updatedWorkflow = updateWorkflowUrls(robot.recording.workflow, targetUrl); robot.set('recording', { ...robot.recording, workflow: updatedWorkflow }); // In the duplicate endpoint const workflow = updateWorkflowUrls(originalRobot.recording.workflow, targetUrl);src/components/robot/RobotEdit.tsx (3)
308-324: Implement consistent URL handling logic.The
handleTargetUrlChangefunction only updates the URL in the last workflow pair, while the server updates all applicable URLs in the workflow. Consider making the client-side behavior match the server-side implementation for consistency.const handleTargetUrlChange = (newUrl: string) => { setRobot((prev) => { if (!prev) return prev; const updatedWorkflow = [...prev.recording.workflow]; - const lastPairIndex = updatedWorkflow.length - 1; - - if (lastPairIndex >= 0) { - const gotoAction = updatedWorkflow[lastPairIndex]?.what?.find(action => action.action === "goto"); - if (gotoAction && gotoAction.args && gotoAction.args.length > 0) { - gotoAction.args[0] = newUrl; - } - } + + // Update all URLs in the workflow, matching server-side behavior + updatedWorkflow.forEach((step) => { + if (step.where?.url && step.where.url !== "about:blank") { + step.where.url = newUrl; + } + + step.what.forEach((action) => { + if (action.action === "goto" && action.args?.length) { + action.args[0] = newUrl; + } + }); + }); return { ...prev, recording: { ...prev.recording, workflow: updatedWorkflow } }; }); };🧰 Tools
🪛 Biome (1.9.4)
[error] 317-317: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
317-318: Use optional chaining consistently.The code can be simplified by using optional chaining consistently.
- const gotoAction = updatedWorkflow[lastPairIndex]?.what?.find(action => action.action === "goto"); - if (gotoAction && gotoAction.args && gotoAction.args.length > 0) { + const gotoAction = updatedWorkflow[lastPairIndex]?.what?.find(action => action.action === "goto"); + if (gotoAction?.args?.length > 0) {🧰 Tools
🪛 Biome (1.9.4)
[error] 317-317: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
410-418: Consider DRY principle for target URL extraction.The URL extraction logic is duplicated across the component. Consider extracting it into a helper function or memoizing the result.
// Add this helper function to the component const getTargetUrl = (workflow: WhereWhatPair[] = []) => { const lastPair = workflow[workflow.length - 1]; return lastPair?.what?.find(action => action.action === "goto")?.args?.[0] || ''; }; // Then use it in both places: // 1. For the TextField value const targetUrl = getTargetUrl(robot?.recording?.workflow); // 2. In the handleSave function const payload = { name: robot.recording_meta.name, limit: robot.recording.workflow[0]?.what[0]?.args?.[0]?.limit, credentials: credentialsForPayload, targetUrl: getTargetUrl(robot.recording.workflow), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/src/routes/storage.ts(2 hunks)src/api/storage.ts(1 hunks)src/components/robot/RobotEdit.tsx(8 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/robot/RobotEdit.tsx
[error] 317-317: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
server/src/routes/storage.ts (2)
262-262: Good addition of the newtargetUrlparameter.The code now properly includes the target URL in the request body destructuring.
296-298: Good use of model update patterns.The code correctly uses
setandchangedmethods to update the model, which ensures proper change tracking.src/api/storage.ts (1)
31-31: Function signature correctly updated to includetargetUrl.The
updateRecordingfunction now properly accepts the new optionaltargetUrlparameter, aligning with the server-side implementation.src/components/robot/RobotEdit.tsx (2)
437-438: Good use of optional chaining in URL extraction.The code effectively uses optional chaining to safely extract the target URL from potentially undefined nested properties.
462-469: Good UI implementation for target URL editing.The new text field for editing the target URL is properly integrated with the component state and handlers.
No description provided.