fix/path param (#2388)

* fix(#484): minor code fixes * code fixes * fixes for generateCode * var change * pr review fixes
usebruno · May 30, 2024 · 470d162 · 470d162
1 parent abfd14a
commit 470d162
Show file tree

Hide file tree

Showing 10 changed files with 88 additions and 90 deletions.
diff --git a/packages/bruno-app/src/components/RequestPane/QueryParams/index.js b/packages/bruno-app/src/components/RequestPane/QueryParams/index.js
@@ -35,22 +35,10 @@ const QueryParams = ({ item, collection }) => {
   const onSave = () => dispatch(saveRequest(item.uid, collection.uid));
   const handleRun = () => dispatch(sendRequest(item, collection.uid));
 
-  const handleValueChange = (data, type, value) => {
-    const _data = cloneDeep(data);
-
-    if (!has(_data, type)) {
-      return;
-    }
-
-    _data[type] = value;
-
-    return _data;
-  };
-
-  const handleQueryParamChange = (e, data, type) => {
+  const handleQueryParamChange = (e, data, key) => {
     let value;
 
-    switch (type) {
+    switch (key) {
       case 'name': {
         value = e.target.value;
         break;
@@ -65,11 +53,17 @@ const QueryParams = ({ item, collection }) => {
       }
     }
 
-    const param = handleValueChange(data, type, value);
+    let queryParam = cloneDeep(data);
+
+    if (queryParam[key] === value) {
+      return;
+    }
+
+    queryParam[key] = value;
 
     dispatch(
       updateQueryParam({
-        param,
+        queryParam,
         itemUid: item.uid,
         collectionUid: collection.uid
       })
@@ -79,11 +73,17 @@ const QueryParams = ({ item, collection }) => {
   const handlePathParamChange = (e, data) => {
     let value = e.target.value;
 
-    const path = handleValueChange(data, 'value', value);
+    let pathParam = cloneDeep(data);
+
+    if (pathParam['value'] === value) {
+      return;
+    }
+
+    pathParam['value'] = value;
 
     dispatch(
       updatePathParam({
-        path,
+        pathParam,
         itemUid: item.uid,
         collectionUid: collection.uid
       })

diff --git a/...pp/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js b/...pp/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js
@@ -27,42 +27,32 @@ const interpolateUrl = ({ url, envVars, collectionVariables, processEnvVars }) =
   });
 };
 
-const joinPathUrl = (url, params) => {
-  const processPaths = (uri, paths) => {
-    return uri
+// interpolate URL paths
+const interpolateUrlPathParams = (url, params) => {
+  const getInterpolatedBasePath = (pathname, params) => {
+    return pathname
       .split('/')
       .map((segment) => {
         if (segment.startsWith(':')) {
-          const paramName = segment.slice(1);
-          const param = paths.find((p) => p.name === paramName && p.type === 'path' && p.enabled);
-          return param ? param.value : segment;
+          const pathParamName = segment.slice(1);
+          const pathParam = params.find((p) => p?.name === pathParamName && p?.type === 'path');
+          return pathParam ? pathParam.value : segment;
         }
         return segment;
       })
       .join('/');
   };
 
-  const processQueryParams = (search, params) => {
-    const queryParams = new URLSearchParams(search);
-    params
-      .filter((p) => p.type === 'query' && p.enabled)
-      .forEach((param) => {
-        queryParams.set(param.name, param.value);
-      });
-    return queryParams.toString();
-  };
-
   let uri;
   try {
     uri = new URL(url);
   } catch (error) {
     uri = new URL(`http://${url}`);
   }
 
-  const basePath = processPaths(uri.pathname, params);
-  const queryString = processQueryParams(uri.search, params);
+  const basePath = getInterpolatedBasePath(uri.pathname, params);
 
-  return `${uri.origin}${basePath}${queryString ? `?${queryString}` : ''}`;
+  return `${uri.origin}${basePath}${uri?.search || ''}`;
 };
 
 const languages = [
@@ -114,10 +104,6 @@ const languages = [
 ];
 
 const GenerateCodeItem = ({ collection, item, onClose }) => {
-  const url = joinPathUrl(
-    get(item, 'draft.request.url') !== undefined ? get(item, 'draft.request.url') : get(item, 'request.url'),
-    get(item, 'draft.request.params') !== undefined ? get(item, 'draft.request.params') : get(item, 'request.params')
-  );
   const environment = findEnvironmentInCollection(collection, collection.activeEnvironmentUid);
   let envVars = {};
   if (environment) {
@@ -128,12 +114,23 @@ const GenerateCodeItem = ({ collection, item, onClose }) => {
     }, {});
   }
 
+  const requestUrl =
+    get(item, 'draft.request.url') !== undefined ? get(item, 'draft.request.url') : get(item, 'request.url');
+
+  // interpolate the query params
   const interpolatedUrl = interpolateUrl({
-    url,
+    url: requestUrl,
     envVars,
     collectionVariables: collection.collectionVariables,
     processEnvVars: collection.processEnvVariables
   });
+
+  // interpolate the path params
+  const finalUrl = interpolateUrlPathParams(
+    interpolatedUrl,
+    get(item, 'draft.request.params') !== undefined ? get(item, 'draft.request.params') : get(item, 'request.params')
+  );
+
   const [selectedLanguage, setSelectedLanguage] = useState(languages[0]);
   return (
     <Modal size="lg" title="Generate Code" handleCancel={onClose} hideFooter={true}>
@@ -157,7 +154,7 @@ const GenerateCodeItem = ({ collection, item, onClose }) => {
             </div>
           </div>
           <div className="flex-grow p-4">
-            {isValidUrl(interpolatedUrl) ? (
+            {isValidUrl(finalUrl) ? (
               <CodeView
                 language={selectedLanguage}
                 item={{
@@ -166,18 +163,18 @@ const GenerateCodeItem = ({ collection, item, onClose }) => {
                     item.request.url !== ''
                       ? {
                           ...item.request,
-                          url: interpolatedUrl
+                          url: finalUrl
                         }
                       : {
                           ...item.draft.request,
-                          url: interpolatedUrl
+                          url: finalUrl
                         }
                 }}
               />
             ) : (
               <div className="flex flex-col justify-center items-center w-full">
                 <div className="text-center">
-                  <h1 className="text-2xl font-bold">Invalid URL: {interpolatedUrl}</h1>
+                  <h1 className="text-2xl font-bold">Invalid URL: {url}</h1>
                   <p className="text-gray-500">Please check the URL and try again</p>
                 </div>
               </div>

diff --git a/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js b/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js
@@ -367,39 +367,42 @@ export const collectionsSlice = createSlice({
           }
           item.draft.request.url = action.payload.url;
 
-          const parts = splitOnFirst(item.draft.request.url, '?');
-          const urlParams = parseQueryParams(parts[1]);
-          let urlPaths = [];
+          const parts = splitOnFirst(item?.draft?.request?.url, '?');
+          const urlQueryParams = parseQueryParams(parts[1]);
+          let urlPathParams = [];
 
           try {
-            urlPaths = parsePathParams(parts[0]);
+            urlPathParams = parsePathParams(parts[0]);
           } catch (err) {
             console.error(err);
             toast.error(err.message);
           }
 
-          const disabledParams = filter(item.draft.request.params, (p) => !p.enabled);
-          let enabledParams = filter(item.draft.request.params, (p) => p.enabled && p.type === 'query');
-          let oldPaths = filter(item.draft.request.params, (p) => p.enabled && p.type === 'path');
-          let newPaths = [];
+          const disabledQueryParams = filter(item?.draft?.request?.params, (p) => !p.enabled && p.type === 'query');
+          let enabledQueryParams = filter(item?.draft?.request?.params, (p) => p.enabled && p.type === 'query');
+          let oldPathParams = filter(item?.draft?.request?.params, (p) => p.enabled && p.type === 'path');
+          let newPathParams = [];
 
           // try and connect as much as old params uid's as possible
-          each(urlParams, (urlParam) => {
-            const existingParam = find(enabledParams, (p) => p.name === urlParam.name || p.value === urlParam.value);
-            urlParam.uid = existingParam ? existingParam.uid : uuid();
-            urlParam.enabled = true;
-            urlParam.type = 'query';
+          each(urlQueryParams, (urlQueryParam) => {
+            const existingQueryParam = find(
+              enabledQueryParams,
+              (p) => p?.name === urlQueryParam?.name || p?.value === urlQueryParam?.value
+            );
+            urlQueryParam.uid = existingQueryParam?.uid || uuid();
+            urlQueryParam.enabled = true;
+            urlQueryParam.type = 'query';
 
             // once found, remove it - trying our best here to accommodate duplicate query params
-            if (existingParam) {
-              enabledParams = filter(enabledParams, (p) => p.uid !== existingParam.uid);
+            if (existingQueryParam) {
+              enabledQueryParams = filter(enabledQueryParams, (p) => p?.uid !== existingQueryParam?.uid);
             }
           });
 
           // filter the newest path param and compare with previous data that already inserted
-          newPaths = filter(urlPaths, (urlPath) => {
-            const existingPath = find(oldPaths, (p) => p.name === urlPath.name);
-            if (existingPath) {
+          newPathParams = filter(urlPathParams, (urlPath) => {
+            const existingPathParam = find(oldPathParams, (p) => p.name === urlPath.name);
+            if (existingPathParam) {
               return false;
             }
             urlPath.uid = uuid();
@@ -409,14 +412,14 @@ export const collectionsSlice = createSlice({
           });
 
           // remove path param that not used or deleted when typing url
-          oldPaths = filter(oldPaths, (urlPath) => {
-            return find(urlPaths, (p) => p.name === urlPath.name);
+          oldPathParams = filter(oldPathParams, (urlPath) => {
+            return find(urlPathParams, (p) => p.name === urlPath.name);
           });
 
           // ultimately params get replaced with params in url + the disabled ones that existed prior
           // the query params are the source of truth, the url in the queryurl input gets constructed using these params
           // we however are also storing the full url (with params) in the url itself
-          item.draft.request.params = concat(urlParams, newPaths, disabledParams, oldPaths);
+          item.draft.request.params = concat(urlQueryParams, newPathParams, disabledQueryParams, oldPathParams);
         }
       }
     },
@@ -491,12 +494,12 @@ export const collectionsSlice = createSlice({
           }
           const queryParam = find(
             item.draft.request.params,
-            (h) => h.uid === action.payload.param.uid && h.type === 'query'
+            (h) => h.uid === action.payload.queryParam.uid && h.type === 'query'
           );
           if (queryParam) {
-            queryParam.name = action.payload.param.name;
-            queryParam.value = action.payload.param.value;
-            queryParam.enabled = action.payload.param.enabled;
+            queryParam.name = action.payload.queryParam.name;
+            queryParam.value = action.payload.queryParam.value;
+            queryParam.enabled = action.payload.queryParam.enabled;
 
             // update request url
             const parts = splitOnFirst(item.draft.request.url, '?');
@@ -558,11 +561,14 @@ export const collectionsSlice = createSlice({
             item.draft = cloneDeep(item);
           }
 
-          const param = find(item.draft.request.params, (p) => p.uid === action.payload.path.uid && p.type === 'path');
+          const param = find(
+            item.draft.request.params,
+            (p) => p.uid === action.payload.pathParam.uid && p.type === 'path'
+          );
 
           if (param) {
-            param.name = action.payload.path.name;
-            param.value = action.payload.path.value;
+            param.name = action.payload.pathParam.name;
+            param.value = action.payload.pathParam.value;
           }
         }
       }

diff --git a/packages/bruno-cli/src/runner/interpolate-vars.js b/packages/bruno-cli/src/runner/interpolate-vars.js
@@ -99,7 +99,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces
       throw { message: 'Invalid URL format', originalError: e.message };
     }
 
-    const urlPaths = url.pathname
+    const interpolatedUrlPath = url.pathname
       .split('/')
       .filter((path) => path !== '')
       .map((path) => {
@@ -113,7 +113,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces
       })
       .join('');
 
-    request.url = url.origin + urlPaths + url.search;
+    request.url = url.origin + interpolatedUrlPath + url.search;
   }
 
   if (request.proxy) {

diff --git a/packages/bruno-cli/src/runner/prepare-request.js b/packages/bruno-cli/src/runner/prepare-request.js
@@ -29,8 +29,7 @@ const prepareRequest = (request, collectionRoot) => {
   let axiosRequest = {
     method: request.method,
     url: request.url,
-    headers: headers,
-    paths: request.paths
+    headers: headers
   };
 
   const collectionAuth = get(collectionRoot, 'request.auth');

diff --git a/packages/bruno-electron/src/ipc/network/interpolate-vars.js b/packages/bruno-electron/src/ipc/network/interpolate-vars.js
@@ -99,7 +99,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces
       throw { message: 'Invalid URL format', originalError: e.message };
     }
 
-    const urlPaths = url.pathname
+    const urlPathnameInterpolatedWithPathParams = url.pathname
       .split('/')
       .filter((path) => path !== '')
       .map((path) => {
@@ -113,7 +113,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces
       })
       .join('');
 
-    request.url = url.origin + urlPaths + url.search;
+    request.url = url.origin + urlPathnameInterpolatedWithPathParams + url.search;
   }
 
   if (request.proxy) {

diff --git a/packages/bruno-lang/v2/src/bruToJson.js b/packages/bruno-lang/v2/src/bruToJson.js
@@ -136,7 +136,7 @@ const mapPairListToKeyValPairs = (pairList = [], parseEnabled = true) => {
   });
 };
 
-const mapPairListToKeyValPairsWithType = (pairList = [], type) => {
+const mapRequestParams = (pairList = [], type) => {
   if (!pairList.length) {
     return [];
   }
@@ -346,17 +346,17 @@ const sem = grammar.createSemantics().addAttribute('ast', {
   },
   query(_1, dictionary) {
     return {
-      params: mapPairListToKeyValPairsWithType(dictionary.ast, 'query')
+      params: mapRequestParams(dictionary.ast, 'query')
     };
   },
   paramspath(_1, dictionary) {
     return {
-      params: mapPairListToKeyValPairsWithType(dictionary.ast, 'path')
+      params: mapRequestParams(dictionary.ast, 'path')
     };
   },
   paramsquery(_1, dictionary) {
     return {
-      params: mapPairListToKeyValPairsWithType(dictionary.ast, 'query')
+      params: mapRequestParams(dictionary.ast, 'query')
     };
   },
   headers(_1, dictionary) {

diff --git a/packages/bruno-schema/src/collections/index.js b/packages/bruno-schema/src/collections/index.js
@@ -185,7 +185,7 @@ const authSchema = Yup.object({
   .noUnknown(true)
   .strict();
 
-const keyValueWithTypeSchema = Yup.object({
+const requestParamsSchema = Yup.object({
   uid: uidSchema,
   name: Yup.string().nullable(),
   value: Yup.string().nullable(),
@@ -203,7 +203,7 @@ const requestSchema = Yup.object({
   url: requestUrlSchema,
   method: requestMethodSchema,
   headers: Yup.array().of(keyValueSchema).required('headers are required'),
-  params: Yup.array().of(keyValueWithTypeSchema).required('params are required'),
+  params: Yup.array().of(requestParamsSchema).required('params are required'),
   auth: authSchema,
   body: requestBodySchema,
   script: Yup.object({