packages/bruno-app/src/utils/collections/index.js (2)

1644-1647: Remove unnecessary async keyword.

The function doesn't contain any await expressions, making the async declaration superfluous. This adds overhead by wrapping the return value in a Promise unnecessarily.

-export const generateUniqueRequestName = async (collection, baseName = 'Untitled', itemUid = null) => {
+export const generateUniqueRequestName = (collection, baseName = 'Untitled', itemUid = null) => {
   if (!collection) {
     return baseName;
   }

---

1649-1652: Move require to top-level import and escape regex special characters.

Using require() inside functions is non-idiomatic for ES modules and incurs repeated resolution overhead. Additionally, if baseName contains regex special characters (e.g., parentheses, dots), the pattern will fail.

 import {cloneDeep, isEqual, sortBy, filter, map, isString, findIndex, find, each, get } from 'lodash';
+import trim from 'lodash/trim';
 // ... other imports

Then update the function:

-  const trim = require('lodash/trim');
   const parentItem = itemUid ? findItemInCollection(collection, itemUid) : null;
   const parentItems = parentItem ? (parentItem.items || []) : (collection.items || []);
-  const baseNamePattern = new RegExp(`^${baseName}(\\d+)?$`);
+  const escapedBaseName = baseName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  const baseNamePattern = new RegExp(`^${escapedBaseName}(\\d+)?$`);

packages/bruno-app/src/components/RequestPane/QueryUrl/index.js (3)

95-107: Inconsistent UX: error toast shown for non-cURL regular paste.

When pasting a regular URL (not a cURL command) into a GraphQL request, this shows an error toast and then allows the paste to proceed. This could confuse users pasting normal URLs.

Consider silently allowing non-cURL pastes without the error:

   const curlCommandRegex = /^\s*curl\s/i;
   if (!curlCommandRegex.test(pastedData)) {
-    toast.error('Invalid cURL command');
     return;
   }

---

178-186: Fix inconsistent indentation.

Lines 181-185 have extra indentation that doesn't match the surrounding code structure, making the control flow harder to follow.

   const curlCommandRegex = /^\s*curl\s/i;
   if (!curlCommandRegex.test(pastedData)) {
     // Not a curl command, allow normal paste behavior
-      return;
-    }
-
-    // Prevent the default paste behavior
-    event.preventDefault();
+    return;
+  }
+
+  // Prevent the default paste behavior
+  event.preventDefault();

---

283-291: TODOs for formUrlEncoded and multipartForm remain unimplemented.

The cURL import silently ignores body content for these modes, which may confuse users expecting full import support.

Consider at minimum showing a toast warning when these body modes are detected but not fully imported. Would you like me to help implement proper param setting for these modes, or open an issue to track this?

packages/bruno-app/src/components/CreateUntitledRequest/index.js (2)

22-116: Unify async handling and reduce duplication across create handlers

All four handleCreate*Request functions follow the same pattern: hide dropdown, compute a unique name, dispatch an async action, then show a success/error toast and optionally call onRequestCreated. Right now they:

• Are marked async but use .then/.catch instead of await
• Duplicate the same success/error handling logic 4 times

Consider refactoring to a shared helper that takes the payload/action creator as input and uses try/catch with await for consistency:

-const handleCreateHttpRequest = async () => {
-  dropdownTippyRef.current?.hide();
-  const uniqueName = await generateUniqueRequestName(collection, 'Untitled', itemUid);
-  const filename = sanitizeName(uniqueName);
-
-  dispatch(
-    newHttpRequest({
-      requestName: uniqueName,
-      filename: filename,
-      requestType: 'http-request',
-      requestUrl: '',
-      requestMethod: 'GET',
-      collectionUid: collection.uid,
-      itemUid: itemUid
-    })
-  )
-    .then(() => {
-      toast.success('New request created!');
-      onRequestCreated?.();
-    })
-    .catch((err) => toast.error(err ? err.message : 'An error occurred while adding the request'));
-};
+const createRequest = async (actionPayloadBuilder) => {
+  dropdownTippyRef.current?.hide();
+  const uniqueName = await generateUniqueRequestName(collection, 'Untitled', itemUid);
+  const filename = sanitizeName(uniqueName);
+
+  try {
+    await dispatch(actionPayloadBuilder({ uniqueName, filename }));
+    toast.success('New request created!');
+    onRequestCreated?.();
+  } catch (err) {
+    toast.error(err ? err.message : 'An error occurred while adding the request');
+  }
+};
+
+const handleCreateHttpRequest = () =>
+  createRequest(({ uniqueName, filename }) =>
+    newHttpRequest({
+      requestName: uniqueName,
+      filename,
+      requestType: 'http-request',
+      requestUrl: '',
+      requestMethod: 'GET',
+      collectionUid: collection.uid,
+      itemUid
+    })
+  );

You can apply the same pattern to the GraphQL, WebSocket, and gRPC handlers to keep their differences (payload shape) focused and avoid repeating the toast logic.

---

10-171: Add a brief JSDoc for the CreateUntitledRequest abstraction

Given this component is now the central entry point for creating new requests of multiple types at the collection root, a short JSDoc on the component describing props (collectionUid, itemUid, onRequestCreated, placement) and behavior would make its purpose clearer to future readers.

Example:

/**
 * Renders a dropdown button that creates new untitled requests (HTTP, GraphQL, WebSocket, gRPC)
 * at the given collection (and optional parent item) using sane defaults.
 *
 * @param {Object} props
 * @param {string} props.collectionUid - Target collection UID.
 * @param {string|null} [props.itemUid=null] - Optional parent item UID within the collection.
 * @param {Function} [props.onRequestCreated] - Callback invoked after a request is successfully created.
 * @param {('top'|'bottom'|'left'|'right')} [props.placement='bottom'] - Dropdown placement.
 */
const CreateUntitledRequest = ({ collectionUid, itemUid = null, onRequestCreated, placement = 'bottom' }) => {

This aligns with your guideline to document abstractions without adding noise.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components should be used as wrappers to define both self and children component styles; Tailwind classes should be used specifically for layout-based styles

packages/bruno-app/src/components/RequestPane/QueryUrl/index.js (1)

166-166: Missing item.type in dependency array.

The callback checks item.type on line 96, but the dependency array only includes item.uid. Add item.type to avoid stale closures if the type changes.

-  }, [dispatch, item.uid, collection.uid]);
+  }, [dispatch, item.uid, item.type, collection.uid]);

packages/bruno-app/src/components/RequestPane/QueryUrl/index.js (2)

283-291: Empty conditional branches with TODO comments.

These empty branches set the body mode but don't populate the params, which may confuse users when their form data doesn't appear. Consider logging a warning or showing a toast indicating partial import.

         } else if (bodyMode === 'formUrlEncoded' && request.body.formUrlEncoded) {
-          // For formUrlEncoded, we need to set each param individually
-          // This is a limitation - we'd need to clear existing params first
-          // For now, we'll set the body mode and the user can manually adjust
-          // TODO: Implement proper formUrlEncoded param setting
+          // TODO: Implement proper formUrlEncoded param setting
+          console.warn('formUrlEncoded body detected but params not fully imported');
         } else if (bodyMode === 'multipartForm' && request.body.multipartForm) {
-          // For multipartForm, similar limitation
-          // TODO: Implement proper multipartForm param setting
+          // TODO: Implement proper multipartForm param setting
+          console.warn('multipartForm body detected but params not fully imported');
         }

Would you like me to open an issue to track the implementation of formUrlEncoded and multipartForm param setting?

---

95-372: Significant code duplication between paste handlers.

Both handleGraphqlPaste and handleHttpPaste share nearly identical logic for URL updates, method updates, and header updates. Consider extracting shared logic into a helper function to reduce maintenance burden.

const applyParsedRequest = (request) => {
  dispatch(requestUrlChanged({ itemUid: item.uid, collectionUid: collection.uid, url: request.url }));
  if (request.method) {
    dispatch(updateRequestMethod({ method: request.method.toUpperCase(), itemUid: item.uid, collectionUid: collection.uid }));
  }
  if (request.headers?.length > 0) {
    dispatch(setRequestHeaders({ collectionUid: collection.uid, itemUid: item.uid, headers: request.headers }));
  }
};

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

packages/bruno-app/src/utils/collections/index.js (1)

1633-1681: Simplify generateUniqueRequestName and align implementation with docs

The logic looks correct for the current Untitled use case, but a few cleanups would make this helper more robust and consistent:

• The function is async but only does synchronous work; dropping async avoids returning a Promise unnecessarily.
• Inside this ES‑module style file, prefer a top‑level import for trim over require('lodash/trim') for consistency and bundler friendliness.
• The JSDoc mentions checking the filesystem, but the current implementation only inspects collection.items; either update the comment or add the filesystem behavior if still needed.
• If you ever pass arbitrary baseName values, consider escaping regex metacharacters in baseNamePattern to avoid surprising matches.

A possible refactor:

-export const generateUniqueRequestName = async (collection, baseName = 'Untitled', itemUid = null) => {
+export const generateUniqueRequestName = (collection, baseName = 'Untitled', itemUid = null) => {
   if (!collection) {
     return baseName;
   }
 
-  const trim = require('lodash/trim');
+  // using lodash.trim via top-level import keeps this helper tree-shakable and consistent with other lodash usage
   const parentItem = itemUid ? findItemInCollection(collection, itemUid) : null;
   const parentItems = parentItem ? (parentItem.items || []) : (collection.items || []);
-  const baseNamePattern = new RegExp(`^${baseName}(\\d+)?$`);
+  const baseNamePattern = new RegExp(`^${baseName}(\\d+)?$`);

And add a top‑level import alongside the other lodash utilities:

import trim from 'lodash/trim';

tests/request/encoding/curl-encoding.spec.ts (1)

2-60: Nice use of createUntitledRequest; consider extracting the Untitled locator

Switching to createUntitledRequest makes these tests much closer to the real user flow and easier to maintain; the subsequent steps to open the generated request and assert code generation look solid.

You might optionally extract the repeated locator for the Untitled request into a variable to improve readability and keep selectors consistent across both tests, e.g.:

const untitledRequest = page.locator('.item-name').filter({ hasText: /^Untitled/ }).first();
await untitledRequest.click();

As per coding guidelines, extracting locator variables where reused keeps Playwright tests easier to read and maintain.

Also applies to: 62-105

tests/collection/create/create-collection.spec.ts (1)

27-34: Tighten URL textarea selector after using createUntitledRequest

The switch to createUntitledRequest looks good here. One small robustness tweak: after clicking #request-url .CodeMirror, using a global page.locator('textarea') can become fragile if another textarea appears in the UI.

You can scope the selector to the URL editor (matching what the helper does) and keep the flow the same:

-    await page.locator('#request-url .CodeMirror').click();
-    await page.locator('textarea').fill('http://localhost:8081');
+    await page.locator('#request-url .CodeMirror').click();
+    await page.locator('#request-url textarea').fill('http://localhost:8081');

Optionally, you could also pass url into createUntitledRequest to avoid duplicating this step, but that’s not required for correctness.

As per coding guidelines, tighter locators help keep Playwright tests stable over time.

tests/collection/moving-requests/tag-persistence.spec.ts (1)

2-69: Avoid the fixed waitForTimeout(1000) before creating untitled requests

The refactor to use createUntitledRequest with tag: 'smoke' and to work purely with the three Untitled requests in the sidebar is solid and reads much better than the manual flow.

The one questionable bit is the explicit await page.waitForTimeout(1000); after saving the collection, especially since each createUntitledRequest already waits for the new-request toast and the later untitledRequests locator asserts toHaveCount(3). If there isn’t a known race here, you can likely drop or replace this with a more targeted wait (e.g., an expect on whatever element indicates the collection is ready) to keep the test fast and less brittle:

-    await page.getByRole('button', { name: 'Save' }).click();
-    await page.waitForTimeout(1000);
+    await page.getByRole('button', { name: 'Save' }).click();
+    // Rely on subsequent `createUntitledRequest` + `toHaveCount(3)` to synchronize instead of a fixed timeout

As per coding guidelines, we try to avoid page.waitForTimeout in Playwright tests unless there’s no reliable locator-based condition to wait on.

tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts (1)

2-62: Stabilize URL and Untitled locators in cross-collection drag-and-drop tests

The move to createUntitledRequest makes both drag‑and‑drop tests much cleaner, and the assertions around presence/absence of Untitled requests in source/target collections look correct.

Two small improvements for robustness/readability:

• As in the collection‑creation test, after clicking #request-url .CodeMirror, scope the textarea to the URL editor to avoid accidentally hitting another textarea:

-    await page.locator('#request-url .CodeMirror').click();
-    await page.locator('textarea').fill('https://echo.usebruno.com');
+    await page.locator('#request-url .CodeMirror').click();
+    await page.locator('#request-url textarea').fill('https://echo.usebruno.com');

• You reuse the .item-name + /^Untitled/ selector multiple times; consider assigning it to a local const (or two, one per collection container) for clarity and to keep selectors consistent.

As per coding guidelines, tighter locators and shared locator variables help keep Playwright tests less flaky.

Also applies to: 64-124

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use 2 spaces for indentation. No tabs, just spaces