From ec32a316a0a0051962b5faf72589e91b47902b6d Mon Sep 17 00:00:00 2001 From: bruno-sachin Date: Wed, 10 Jun 2026 18:12:19 +0530 Subject: [PATCH 1/3] BRU-3172 Collection docs Incorrectly mark path parameters as query parameters --- .../oc-docs/src/components/Docs/Item/Item.tsx | 26 ++- .../DrawerContent/Views/Common/ParamsTab.tsx | 143 +++++++++++--- .../Views/PlaygroundView/PlaygroundView.tsx | 2 +- .../PlaygroundView/QueryBar/QueryBar.tsx | 13 +- .../RequestPane/RequestPane.tsx | 8 +- .../oc-docs/src/runner/RequestExecutor.ts | 7 +- .../oc-docs/src/store/slices/playground.ts | 2 +- .../src/ui/KeyValueTable/KeyValueTable.css | 15 ++ .../src/ui/KeyValueTable/KeyValueTable.tsx | 94 +++++---- packages/oc-docs/src/utils/pathParams.spec.ts | 184 ++++++++++++++++++ packages/oc-docs/src/utils/pathParams.ts | 141 ++++++++++++++ 11 files changed, 553 insertions(+), 82 deletions(-) create mode 100644 packages/oc-docs/src/utils/pathParams.spec.ts create mode 100644 packages/oc-docs/src/utils/pathParams.ts diff --git a/packages/oc-docs/src/components/Docs/Item/Item.tsx b/packages/oc-docs/src/components/Docs/Item/Item.tsx index 9acd246..1805e1c 100644 --- a/packages/oc-docs/src/components/Docs/Item/Item.tsx +++ b/packages/oc-docs/src/components/Docs/Item/Item.tsx @@ -5,7 +5,7 @@ import 'prismjs/components/prism-graphql'; import 'prismjs/components/prism-json'; import 'prismjs/components/prism-xml-doc'; import 'prismjs/components/prism-python'; -import type { HttpRequest } from '@opencollection/types/requests/http'; +import type { HttpRequest, HttpRequestParam } from '@opencollection/types/requests/http'; import type { Variable } from '@opencollection/types/common/variables'; import { generateSectionId, getItemId } from '../../../utils/itemUtils'; import { @@ -24,7 +24,6 @@ import { getRequestExamples, scriptsArrayToObject, isFolder, - isHttpRequest } from '../../../utils/schemaHelpers'; import { MinimalDataTable, @@ -218,6 +217,14 @@ const Item = memo(({ examples }; + // Query and path params share one `params` array (distinguished by `type`); + // split in a single pass so each renders in its own labelled table. + const queryParams: HttpRequestParam[] = []; + const pathParams: HttpRequestParam[] = []; + for (const param of endpoint.params || []) { + (param?.type === 'path' ? pathParams : queryParams).push(param); + } + return (
- {endpoint.params && endpoint.params.length > 0 && ( + {queryParams.length > 0 && ( )} + {pathParams.length > 0 && ( + + )} + {endpoint.headers && endpoint.headers.length > 0 && ( void; + keyLabel?: string; + showEnabled?: boolean; + showActions?: boolean; + disableNewRow?: boolean; + readOnlyKey?: boolean; +} + +/** + * A single labelled key/value table. Memoized so that editing one params table + * (e.g. query) doesn't force the sibling table (e.g. path) to re-render while + * its data/handler references are unchanged. + */ +const ParamsSection: React.FC = React.memo(({ + title, + description, + data, + onChange, + keyLabel = 'Key', + showEnabled = true, + showActions = true, + disableNewRow = false, + readOnlyKey = false +}) => ( +
+
+ + {title} + + {description && ( + + {description} + + )} +
+ +
+)); + +ParamsSection.displayName = 'ParamsSection'; + export const ParamsTab: React.FC = ({ params, onParamsChange, - title = "Query Parameters", + title = 'Query', description }) => { - const paramsData: KeyValueRow[] = (params || []).map((param, index) => ({ - id: `param-${index}`, - name: param.name || '', - value: param.value || '', - enabled: !param.disabled, - type: param.type || 'query' - })); + // Single pass: split the params into query and path rows. `type` is kept on + // each row so it survives the round-trip back through onParamsChange. + const { queryData, pathData } = useMemo(() => { + const queryData: KeyValueRow[] = []; + const pathData: KeyValueRow[] = []; + + (params || [])?.forEach((param, index) => { + const row: KeyValueRow = { + id: `param-${index}`, + name: param.name || '', + value: param.value || '', + enabled: !param.disabled, + type: param.type || 'query' + }; + (row.type === 'path' ? pathData : queryData).push(row); + }); + + return { queryData, pathData }; + }, [params]); + + // Each table only owns its own subset, so on change we re-tag the edited rows + // and merge them back with the untouched sibling rows before bubbling up. + const handleQueryChange = useCallback( + (rows: KeyValueRow[]) => { + const queryRows = rows?.map((row) => ({ ...row, type: 'query' })); + onParamsChange([...queryRows, ...pathData]); + }, + [onParamsChange, pathData] + ); + + const handlePathChange = useCallback( + (rows: KeyValueRow[]) => { + const pathRows = rows?.map((row) => ({ ...row, type: 'path' })); + onParamsChange([...queryData, ...pathRows]); + }, + [onParamsChange, queryData] + ); + + const hasPath = pathData.length > 0; return ( -
-
- - {title} - - {description && ( - - {description} - - )} -
- + {/* Query table: always shown so query params can be viewed/added + regardless of whether the request currently has any. */} + + + {/* Path table: only shown when the URL actually defines path params. */} + {hasPath && ( + + )}
); }; diff --git a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/PlaygroundView.tsx b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/PlaygroundView.tsx index bd457d1..4e400ae 100644 --- a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/PlaygroundView.tsx +++ b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/PlaygroundView.tsx @@ -83,7 +83,7 @@ const Playground: React.FC = ({ item, collection, selectedEnvir } finally { setIsLoading(false); } - }, [collection, editableItem, runner, selectedEnvironment]); + }, [collection, editableItem, runner, selectedEnvironment, itemUuid]); const handleMouseDown = useCallback((e: React.MouseEvent) => { setIsDragging(true); diff --git a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx index 916c074..f817175 100644 --- a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx +++ b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx @@ -1,7 +1,8 @@ import React, { useState, useEffect } from 'react'; import type { HttpRequest } from '@opencollection/types/requests/http'; import { StyledWrapper } from './StyledWrapper'; -import { getHttpMethod, getRequestUrl } from '../../../../../../utils/schemaHelpers'; +import { getHttpMethod, getRequestUrl, getHttpParams } from '../../../../../../utils/schemaHelpers'; +import { syncPathParams } from '../../../../../../utils/pathParams'; interface QueryBarProps { item: HttpRequest; @@ -21,11 +22,19 @@ const QueryBar: React.FC = ({ item, onSendRequest, isLoading, onI const handleUrlChange = (newUrl: string) => { setUrl(newUrl); + + // Keep the path params in sync with the URL's `:name` segments (add on type, + // remove on delete). + const currentParams = getHttpParams(item); + const syncedParams = syncPathParams(currentParams, newUrl); + console.log('syncedParams', syncedParams); + const updatedItem = { ...item, http: { ...item.http, - url: newUrl + url: newUrl, + ...(syncedParams !== currentParams ? { params: syncedParams } : {}) } }; onItemChange(updatedItem); diff --git a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/RequestPane/RequestPane.tsx b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/RequestPane/RequestPane.tsx index 4b4a161..a390bf3 100644 --- a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/RequestPane/RequestPane.tsx +++ b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/RequestPane/RequestPane.tsx @@ -26,10 +26,10 @@ const RequestPane: React.FC = ({ item, onItemChange }) => { const handleParamsChange = (params: KeyValueRow[]) => { const updatedParams = params.map(p => ({ - name: p.name, - value: p.value, - disabled: !p.enabled, - type: 'query' as const + name: p?.name, + value: p?.value, + disabled: !p?.enabled, + type: p?.type })); onItemChange({ ...item, diff --git a/packages/oc-docs/src/runner/RequestExecutor.ts b/packages/oc-docs/src/runner/RequestExecutor.ts index 4e52326..69614f9 100644 --- a/packages/oc-docs/src/runner/RequestExecutor.ts +++ b/packages/oc-docs/src/runner/RequestExecutor.ts @@ -1,6 +1,7 @@ import type { HttpRequest } from '@opencollection/types/requests/http'; import { RunRequestResponse } from './index'; -import { getHttpMethod, getRequestUrl, getHttpHeaders, getHttpBody, getRequestAuth } from '../utils/schemaHelpers'; +import { getHttpMethod, getRequestUrl, getHttpHeaders, getHttpBody, getRequestAuth, getHttpParams } from '../utils/schemaHelpers'; +import { applyPathParams } from '../utils/pathParams'; import stripJsonComments from 'strip-json-comments'; export class RequestExecutor { @@ -9,7 +10,9 @@ export class RequestExecutor { try { const fetchOptions = await this.buildFetchOptions(request, options.timeout); - const requestUrl = getRequestUrl(request); + // Substitute `:name` path params (e.g. /posts/:postId -> /posts/1) before + // sending. Values are already variable-interpolated by this point. + const requestUrl = applyPathParams(getRequestUrl(request), getHttpParams(request)); const response = await fetch(requestUrl, fetchOptions); const endTime = Date.now(); diff --git a/packages/oc-docs/src/store/slices/playground.ts b/packages/oc-docs/src/store/slices/playground.ts index f7a3e33..cea366b 100644 --- a/packages/oc-docs/src/store/slices/playground.ts +++ b/packages/oc-docs/src/store/slices/playground.ts @@ -4,7 +4,7 @@ import type { Item as OpenCollectionItem, Folder } from '@opencollection/types/c import type { HttpRequest } from '@opencollection/types/requests/http'; import type { RootState } from '../store'; import { hydrateWithUUIDs, findAndUpdateItem } from '../../utils/items'; -import { isFolder, getItemType } from '../../utils/schemaHelpers'; +import { isFolder } from '../../utils/schemaHelpers'; export type ViewMode = 'playground' | 'environments' | 'folder-settings' | 'collection-settings'; diff --git a/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.css b/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.css index 913189f..f985dfc 100644 --- a/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.css +++ b/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.css @@ -114,6 +114,21 @@ outline: none; } +/* Read-only key cell: matches the .text-input box model for column alignment, + but renders as static, non-editable text. */ +.key-value-table .text-readonly { + display: block; + width: 100%; + border: 1px solid transparent; + padding: 5px 8px; + font-size: 12px; + color: var(--text-primary); + font-family: inherit; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} + .key-value-table .text-input::placeholder { color: var(--text-tertiary); opacity: 0.6; diff --git a/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.tsx b/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.tsx index 93b453b..46e2281 100644 --- a/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.tsx +++ b/packages/oc-docs/src/ui/KeyValueTable/KeyValueTable.tsx @@ -23,6 +23,8 @@ interface KeyValueTableProps { className?: string; disableNewRow?: boolean; disableDelete?: boolean; + showActions?: boolean; + readOnlyKey?: boolean; } const generateId = () => `row_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; @@ -36,7 +38,9 @@ const KeyValueTable: React.FC = ({ additionalColumns = [], className = '', disableNewRow = false, - disableDelete = false + disableDelete = false, + showActions = true, + readOnlyKey = false }) => { const isEditingRef = useRef(false); const focusRef = useRef<{ index: number; field: 'name' | 'value' } | null>(null); @@ -207,7 +211,7 @@ const KeyValueTable: React.FC = ({ {additionalColumns.map(col => ( {col.label} ))} - + {showActions && } @@ -231,20 +235,24 @@ const KeyValueTable: React.FC = ({ )} - { - inputRefs.current[`${index}-name`] = el; - }} - type="text" - className="text-input" - value={row.name} - placeholder={isLastEmptyRow ? keyPlaceholder : ''} - onChange={(e) => handleFieldChange(index, 'name', e.target.value)} - autoComplete="off" - autoCorrect="off" - autoCapitalize="off" - spellCheck={false} - /> + {readOnlyKey ? ( + {row.name} + ) : ( + { + inputRefs.current[`${index}-name`] = el; + }} + type="text" + className="text-input" + value={row.name} + placeholder={isLastEmptyRow ? keyPlaceholder : ''} + onChange={(e) => handleFieldChange(index, 'name', e.target.value)} + autoComplete="off" + autoCorrect="off" + autoCapitalize="off" + spellCheck={false} + /> + )} = ({ {!isLastEmptyRow && col.render(row, index)} ))} - - {!isLastEmptyRow && !disableDelete && ( - - )} - + + + + + + + + )} + + )} ); })} diff --git a/packages/oc-docs/src/utils/pathParams.spec.ts b/packages/oc-docs/src/utils/pathParams.spec.ts new file mode 100644 index 0000000..8b7014d --- /dev/null +++ b/packages/oc-docs/src/utils/pathParams.spec.ts @@ -0,0 +1,184 @@ +import { describe, it, expect } from 'vitest'; +import { parsePathParamNames, syncPathParams, applyPathParams } from './pathParams'; + +describe('parsePathParamNames', () => { + it('extracts a single path param', () => { + expect(parsePathParamNames('https://api.com/posts/:postId')).toEqual(['postId']); + }); + + it('extracts multiple path params in order', () => { + expect(parsePathParamNames('{{host}}/users/:userId/posts/:postId')).toEqual([ + 'userId', + 'postId' + ]); + }); + + it('ignores the query string and fragment', () => { + expect(parsePathParamNames('/posts/:postId?expand=:nope#:frag')).toEqual(['postId']); + }); + + it('does not treat the protocol or port colon as a param', () => { + expect(parsePathParamNames('http://localhost:8081/api/:id')).toEqual(['id']); + }); + + it('ignores a bare colon with no name', () => { + expect(parsePathParamNames('/posts/:')).toEqual([]); + }); + + it('stops the name at the first non-identifier char', () => { + expect(parsePathParamNames('/posts/:postId.json')).toEqual(['postId']); + }); + + it('de-duplicates repeated names, keeping first-seen order', () => { + expect(parsePathParamNames('/a/:id/b/:id')).toEqual(['id']); + }); + + it('leaves variable templates untouched', () => { + expect(parsePathParamNames('{{baseUrl}}/v1/:id')).toEqual(['id']); + }); + + it('handles empty / non-string input', () => { + expect(parsePathParamNames('')).toEqual([]); + expect(parsePathParamNames(undefined)).toEqual([]); + expect(parsePathParamNames(null)).toEqual([]); + }); +}); + +describe('syncPathParams', () => { + const query = (name: string, value = ''): any => ({ name, value, type: 'query' }); + const path = (name: string, value = ''): any => ({ name, value, type: 'path' }); + + it('adds a path param when the URL gains a :segment', () => { + const result = syncPathParams([], 'https://api.com/posts/:postId'); + expect(result).toEqual([{ name: 'postId', value: '', type: 'path' }]); + }); + + it('removes a path param when the URL drops the :segment', () => { + const result = syncPathParams([path('postId', '1')], 'https://api.com/posts'); + expect(result).toEqual([]); + }); + + it('preserves the edited value of a path param that stays in the URL', () => { + const existing = [path('postId', '42')]; + const result = syncPathParams(existing, 'https://api.com/posts/:postId'); + expect(result).toBe(existing); // reference-stable, value kept + expect(result[0].value).toBe('42'); + }); + + it('never touches query params', () => { + const result = syncPathParams( + [query('q', 'alice'), path('postId', '1')], + 'https://api.com/posts/:postId/comments/:commentId' + ); + expect(result).toEqual([ + { name: 'q', value: 'alice', type: 'query' }, + { name: 'postId', value: '1', type: 'path' }, // value preserved + { name: 'commentId', value: '', type: 'path' } // newly added + ]); + }); + + it('returns the same reference when nothing changes', () => { + const existing = [query('q', 'alice')]; + expect(syncPathParams(existing, 'https://api.com/search')).toBe(existing); + }); + + it('returns the same reference when only the query string changes', () => { + const existing = [path('id', '7')]; + expect(syncPathParams(existing, 'https://api.com/x/:id?foo=bar')).toBe(existing); + }); + + it('reorders path params to match the URL when the set changes', () => { + const result = syncPathParams( + [path('postId', '1'), path('userId', '2')], + '{{host}}/users/:userId/posts/:postId' + ); + expect(result.map((p) => p.name)).toEqual(['userId', 'postId']); + // values still matched by name + expect(result.find((p) => p.name === 'postId')?.value).toBe('1'); + expect(result.find((p) => p.name === 'userId')?.value).toBe('2'); + }); + + it('handles undefined params', () => { + expect(syncPathParams(undefined, 'https://api.com/posts/:postId')).toEqual([ + { name: 'postId', value: '', type: 'path' } + ]); + }); +}); + +describe('applyPathParams', () => { + const path = (name: string, value: string, extra: object = {}): any => ({ + name, + value, + type: 'path', + ...extra + }); + const query = (name: string, value: string): any => ({ name, value, type: 'query' }); + + it('substitutes a single path param', () => { + expect(applyPathParams('https://api.com/posts/:postId', [path('postId', '1')])).toBe( + 'https://api.com/posts/1' + ); + }); + + it('substitutes multiple path params', () => { + expect( + applyPathParams('{{host}}/users/:userId/posts/:postId', [ + path('userId', '7'), + path('postId', '99') + ]) + ).toBe('{{host}}/users/7/posts/99'); + }); + + it('leaves the query string untouched and keeps it on the result', () => { + expect( + applyPathParams('https://api.com/posts/:postId?expand=true', [path('postId', '1')]) + ).toBe('https://api.com/posts/1?expand=true'); + }); + + it('never confuses the port or protocol with a path param', () => { + expect(applyPathParams('http://localhost:8081/api/:id', [path('id', '5')])).toBe( + 'http://localhost:8081/api/5' + ); + }); + + it('URL-encodes values by default', () => { + expect(applyPathParams('/files/:name', [path('name', 'a b/c')])).toBe('/files/a%20b%2Fc'); + }); + + it('does not encode when encode=false', () => { + expect(applyPathParams('/files/:name', [path('name', 'a b')], { encode: false })).toBe( + '/files/a b' + ); + }); + + it('preserves a non-identifier suffix on the segment', () => { + expect(applyPathParams('/posts/:postId.json', [path('postId', '1')])).toBe('/posts/1.json'); + }); + + it('leaves a :segment untouched when no matching param exists', () => { + expect(applyPathParams('/posts/:postId', [query('q', 'x')])).toBe('/posts/:postId'); + }); + + it('skips disabled path params (leaves the segment literal)', () => { + expect(applyPathParams('/posts/:postId', [path('postId', '1', { disabled: true })])).toBe( + '/posts/:postId' + ); + }); + + it('replaces every occurrence of a repeated param', () => { + expect(applyPathParams('/a/:id/b/:id', [path('id', 'X')])).toBe('/a/X/b/X'); + }); + + it('returns the url unchanged when there are no path params', () => { + const url = 'https://api.com/search?q=alice'; + expect(applyPathParams(url, [query('q', 'alice')])).toBe(url); + expect(applyPathParams(url, [])).toBe(url); + expect(applyPathParams(url, undefined)).toBe(url); + }); + + it('handles empty / nullish urls', () => { + expect(applyPathParams('', [path('id', '1')])).toBe(''); + expect(applyPathParams(undefined, [path('id', '1')])).toBe(''); + expect(applyPathParams(null, [path('id', '1')])).toBe(''); + }); +}); diff --git a/packages/oc-docs/src/utils/pathParams.ts b/packages/oc-docs/src/utils/pathParams.ts new file mode 100644 index 0000000..7735869 --- /dev/null +++ b/packages/oc-docs/src/utils/pathParams.ts @@ -0,0 +1,141 @@ +import type { HttpRequestParam } from '@opencollection/types/requests/http'; + +/** + * Extract the ordered, de-duplicated list of path-parameter names declared in a + * URL using the `:name` segment syntax (e.g. `/posts/:postId` -> `['postId']`). + * + * Rules (mirroring the request-client behaviour): + * - Only a segment that *starts* with `:` is a path param, so ports + * (`localhost:8081`) and protocols (`https://`) are never misread as params. + * - The query string and fragment are ignored — only the path is scanned. + * - The name is the identifier directly after the colon (`[A-Za-z0-9_-]+`), so + * `:postId.json` yields `postId` and a bare `:` yields nothing. + * - Variable templates such as `{{host}}` are left untouched. + * - Repeated names collapse to a single entry, preserving first-seen order. + */ +export const parsePathParamNames = (url: string | undefined | null): string[] => { + if (!url || typeof url !== 'string') return []; + + // Only the path matters; drop the query string and fragment up-front. + const pathPortion = url.split('?')[0].split('#')[0]; + + const names: string[] = []; + const seen = new Set(); + + for (const segment of pathPortion.split('/')) { + // Fast reject before running the regex: must start with ':' and have more. + if (segment.length < 2 || segment[0] !== ':') continue; + + const match = /^:([A-Za-z0-9_-]+)/.exec(segment); + if (!match) continue; + + const name = match[1]; + if (!seen.has(name)) { + seen.add(name); + names.push(name); + } + } + + return names; +}; + +/** + * Reconcile a request's `params` with the path params declared in its URL. + * + * Query params (anything not typed `'path'`) are preserved untouched. Path + * params are rebuilt from the URL's `:name` segments: existing path params are + * reused by name (so edited values survive), names no longer in the URL are + * dropped, and new names are added with an empty value. + * + * Returns the original `params` reference when nothing changed, so callers can + * cheaply skip redundant state updates / re-renders. + */ +export const syncPathParams = ( + params: HttpRequestParam[] | undefined, + url: string +): HttpRequestParam[] => { + const existing = params ?? []; + const pathNames = parsePathParamNames(url); + const existingPath = existing.filter((p) => p?.type === 'path'); + + // Nothing references path params on either side — leave the array as-is. + if (pathNames.length === 0 && existingPath.length === 0) { + return existing; + } + + // Reuse existing path params (first match per name) to keep edited values. + const existingByName = new Map(); + for (const p of existingPath) { + if (!existingByName.has(p.name)) existingByName.set(p.name, p); + } + + const nextPath: HttpRequestParam[] = pathNames.map( + (name) => existingByName.get(name) ?? { name, value: '', type: 'path' as const } + ); + + // Reference-stable when the path set is identical (same entries, same order). + const unchanged = + existingPath.length === nextPath.length && + existingPath.every((p, i) => p === nextPath[i]); + if (unchanged) { + return existing; + } + + // Keep query params (and their order); swap in the reconciled path set. + const queryParams = existing.filter((p) => p?.type !== 'path'); + return [...queryParams, ...nextPath]; +}; + +/** + * Replace `:name` path segments in a URL with the values of the matching + * (enabled) path params — e.g. `/posts/:postId` + `{ postId: '1' }` -> `/posts/1`. + * + * - Only the path is rewritten; the query string and fragment are left intact + * (so a `:` inside a query value is never touched). + * - The segment's name is the identifier after the colon, matching + * `parsePathParamNames`, so `:postId.json` -> `.json`. + * - Values are URL-encoded by default so they are safe, single path segments. + * - A `:name` with no matching/enabled param (or no value provided) is left + * as-is rather than producing a broken URL. + */ +export const applyPathParams = ( + url: string | undefined | null, + params: HttpRequestParam[] | undefined, + options: { encode?: boolean } = {} +): string => { + if (!url || typeof url !== 'string') return url ?? ''; + + const { encode = true } = options; + + // Map enabled path params by name (first occurrence wins). + const valueByName = new Map(); + for (const p of params ?? []) { + if (p?.type === 'path' && !p.disabled && p.name && !valueByName.has(p.name)) { + valueByName.set(p.name, p.value ?? ''); + } + } + if (valueByName.size === 0) return url; + + // Substitute only within the path; preserve the query string / fragment. + const sepIndex = url.search(/[?#]/); + const pathPart = sepIndex === -1 ? url : url.slice(0, sepIndex); + const rest = sepIndex === -1 ? '' : url.slice(sepIndex); + + const newPath = pathPart + .split('/') + .map((segment) => { + if (segment.length < 2 || segment[0] !== ':') return segment; + + const match = /^:([A-Za-z0-9_-]+)(.*)$/.exec(segment); + if (!match) return segment; + + const [, name, suffix] = match; + if (!valueByName.has(name)) return segment; + + const value = valueByName.get(name) as string; + return (encode ? encodeURIComponent(value) : value) + suffix; + }) + .join('/'); + + return newPath + rest; +}; From 23dfe1f141d93db1afcc0e0567e0c52671d7d3fd Mon Sep 17 00:00:00 2001 From: bruno-sachin Date: Thu, 11 Jun 2026 12:19:58 +0530 Subject: [PATCH 2/3] Code Refactoring --- .../Views/PlaygroundView/QueryBar/QueryBar.tsx | 1 - packages/oc-docs/src/runner/RequestExecutor.ts | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx index f817175..36e0da7 100644 --- a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx +++ b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/QueryBar/QueryBar.tsx @@ -27,7 +27,6 @@ const QueryBar: React.FC = ({ item, onSendRequest, isLoading, onI // remove on delete). const currentParams = getHttpParams(item); const syncedParams = syncPathParams(currentParams, newUrl); - console.log('syncedParams', syncedParams); const updatedItem = { ...item, diff --git a/packages/oc-docs/src/runner/RequestExecutor.ts b/packages/oc-docs/src/runner/RequestExecutor.ts index 69614f9..63b18f0 100644 --- a/packages/oc-docs/src/runner/RequestExecutor.ts +++ b/packages/oc-docs/src/runner/RequestExecutor.ts @@ -1,6 +1,6 @@ import type { HttpRequest } from '@opencollection/types/requests/http'; import { RunRequestResponse } from './index'; -import { getHttpMethod, getRequestUrl, getHttpHeaders, getHttpBody, getRequestAuth, getHttpParams } from '../utils/schemaHelpers'; +import { getHttpMethod, getRequestUrl, getHttpHeaders, getHttpBody, getRequestAuth, getHttpParams, getRequestSettings } from '../utils/schemaHelpers'; import { applyPathParams } from '../utils/pathParams'; import stripJsonComments from 'strip-json-comments'; @@ -11,8 +11,11 @@ export class RequestExecutor { try { const fetchOptions = await this.buildFetchOptions(request, options.timeout); // Substitute `:name` path params (e.g. /posts/:postId -> /posts/1) before - // sending. Values are already variable-interpolated by this point. - const requestUrl = applyPathParams(getRequestUrl(request), getHttpParams(request)); + // sending. Values are already variable-interpolated by this point. Honour + // the request's encodeUrl setting — only an explicit `false` disables + // encoding (undefined / 'inherit' keep the safe default of encoding). + const encodeUrl = getRequestSettings(request)?.encodeUrl !== false; + const requestUrl = applyPathParams(getRequestUrl(request), getHttpParams(request), { encode: encodeUrl }); const response = await fetch(requestUrl, fetchOptions); const endTime = Date.now(); From d072dbdc0c7077c9d291de29a0f2880753ba81bd Mon Sep 17 00:00:00 2001 From: bruno-sachin Date: Fri, 12 Jun 2026 11:59:32 +0530 Subject: [PATCH 3/3] Comments addressed --- .../PlaygroundDrawer/DrawerContent/Views/Common/ParamsTab.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/Common/ParamsTab.tsx b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/Common/ParamsTab.tsx index ef09afc..c7d7e98 100644 --- a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/Common/ParamsTab.tsx +++ b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/Common/ParamsTab.tsx @@ -92,7 +92,7 @@ export const ParamsTab: React.FC = ({ // and merge them back with the untouched sibling rows before bubbling up. const handleQueryChange = useCallback( (rows: KeyValueRow[]) => { - const queryRows = rows?.map((row) => ({ ...row, type: 'query' })); + const queryRows = (rows ?? []).map((row) => ({ ...row, type: 'query' })); onParamsChange([...queryRows, ...pathData]); }, [onParamsChange, pathData] @@ -100,7 +100,7 @@ export const ParamsTab: React.FC = ({ const handlePathChange = useCallback( (rows: KeyValueRow[]) => { - const pathRows = rows?.map((row) => ({ ...row, type: 'path' })); + const pathRows = (rows ?? []).map((row) => ({ ...row, type: 'path' })); onParamsChange([...queryData, ...pathRows]); }, [onParamsChange, queryData]