Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: path parameters #484

Merged

Conversation

game5413
Copy link
Contributor

@game5413 game5413 commented Oct 9, 2023

Currently, for setting URL path parameter that have dynamic value such as ID only available with two options. First changing directly the URL path and using Vars Pre-Request or Collection Variables, on this PR add new section to manage path parameters for user to changing value alongside with query params at one a time or individually. Also to provide solution for #281 issue.

@game5413
Copy link
Contributor Author

game5413 commented Oct 9, 2023

@helloanoop can you please help me to check any place that i forgot to add path params ?

@helloanoop
Copy link
Contributor

This is great! Long PR. Will get this in for the next release.

game5413 added 2 commits October 13, 2023 21:42
# Conflicts:
#	packages/bruno-app/src/components/RequestPane/HttpRequestPane/index.js
#	packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js
#	packages/bruno-lang/v2/src/bruToJson.js
@helloanoop helloanoop added this to the v1 milestone Oct 18, 2023
# Conflicts:
#	packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js
#	packages/bruno-electron/src/ipc/network/prepare-request.js
@gianpo86
Copy link
Contributor

@game5413 Looking forward to see this feature, your branch has conflicts with main, could you check it please?

# Conflicts:
#	packages/bruno-lang/v2/src/bruToJson.js
Comment on lines +106 to +113
<table>
<thead>
<tr>
<td>Name</td>
<td>Value</td>
<td></td>
</tr>
</thead>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Query and Param tables are very similar to each other. It might be worth extracting a component for them.

Props would be something like

  • items
  • readOnlyKeys
  • onChangeKey
  • onChangeValue
  • onAddRow
  • onRemoveRow

@@ -25,6 +25,53 @@ const interpolateUrl = ({ url, envVars, collectionVariables, processEnvVars }) =
});
};

const joinPathUrl = (url, paths) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider exporting this function and writing unit tests for it

@1Mark
Copy link

1Mark commented Dec 29, 2023

Can't wait for this to be merged 🎉

@@ -14,6 +20,7 @@ const QueryParams = ({ item, collection }) => {
const dispatch = useDispatch();
const { storedTheme } = useTheme();
const params = item.draft ? get(item, 'draft.request.params') : get(item, 'request.params');
const paths = item.draft ? get(item, 'draft.request.paths') : get(item, 'request.paths');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am looking into this PR as I am very interested in having this feature in Bruno, sadly in no way am I capable of reviewing JS code.
However, regarding these changes, I would fear ambiguity (in future) coming from variable names.
To avoid it, you could introduce changes similar to the following idea:

  • [dir] QueryParams -> RequestParams
  • [var] params -> queryParams
  • [var] paths -> pathParams

@benjaminrae
Copy link

Any updates on this? Or any help required? This is the only feature that is stopping me from leaving Postman behind and adopting Bruno

# Conflicts:
#	docs/readme/readme_fr.md
#	packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js
#	packages/bruno-electron/src/ipc/network/prepare-request.js
#	readme.md
@DrGrognon
Copy link

Hello! I'm also super excited by this feature 😃 @game5413 is there any way we can help you on this PR ?

# Conflicts:
#	contributing.md
#	packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js
#	packages/bruno-cli/src/runner/interpolate-vars.js
#	packages/bruno-electron/src/ipc/network/interpolate-vars.js
@Fle9ma
Copy link

Fle9ma commented Feb 16, 2024

Hi 👋🏻 I'm so happy about this that I can't wait for this PR to be merged, if I can help in any way, please let me know.

@andrewgee
Copy link

We'd really like this to be introduced to bruno as well! It's the one thing that's keeping us using Postman at the moment

@o-ric
Copy link

o-ric commented Mar 21, 2024

I'm also super excited by this feature, and this is also the missing feature that is stopping my team from adopting Bruno definitively.
But it seems that something is blocking this feature, as it has been in development for months now.
Any news on this ?

@helloanoop commented on Oct 13, 2023

This is great! Long PR. Will get this in for the next release.

@pho3nixf1re
Copy link

Looks like this has gone stale with time due to conflicts and failing tests. Is @game5413 OK with me cleaning this up and opening a new PR? Alternatively I could PR your branch for you to then update here.

@gianpo86
Copy link
Contributor

gianpo86 commented Apr 2, 2024 via email

@helloanoop
Copy link
Contributor

Update: @lohxt1 is reviewing this. We will have this shipped by the end of this week.

@sanjai0py sanjai0py self-assigned this May 11, 2024
@helloanoop helloanoop changed the base branch from main to feat/path-parameters May 13, 2024 10:10
@helloanoop helloanoop merged commit 2e71e60 into usebruno:feat/path-parameters May 13, 2024
@helloanoop
Copy link
Contributor

helloanoop commented May 13, 2024

I've merged this to a feature branch on Bruno's main repo.
We have to do some minor updates in the schema and then this should be GTG.

@sanjai0py please have the updates done in the next 2 days as discussed.

helloanoop added a commit that referenced this pull request May 30, 2024
* feat: path parameters (#484)

* add path parameters on bruno-app

* add path parameters on bruno-cli

* fix bruno-schema testing

* fix generate request code not replace path parameter value

---------

Co-authored-by: game5413 <febryanph10@gmail.com>
Co-authored-by: Anoop M D <anoop.md1421@gmail.com>

* feat: Refactor request parameter handling

- Update prepare-request.js to filter and rename 'paths' to 'params' with type 'path'
- Remove 'paths' from export.js and interpolate-vars.js
- Update bru.js to use 'params' instead of 'path'
- Update requestSchema in index.js to use 'keyValueWithTypeSchema' for 'params'

Co-authored-by: game5413 <febryanph10@gmail.com>
Co-authored-by: Anoop M D <anoop.md1421@gmail.com>

* feat: Refactor request parameter handling

* refactor: changes form the review

* refactor: Refactor transformItemsInCollection handling

* refactor: Refactor improved export/import functionalities

* refactor: Remove console.log statement in bruToJson.js

---------

Co-authored-by: game5413 <37659721+game5413@users.noreply.github.com>
Co-authored-by: game5413 <febryanph10@gmail.com>
Co-authored-by: Anoop M D <anoop.md1421@gmail.com>
lohxt1 added a commit to lohxt1/bruno that referenced this pull request May 30, 2024
helloanoop pushed a commit that referenced this pull request May 30, 2024
* fix(#484): minor code fixes

* code fixes

* fixes for generateCode

* var change

* pr review fixes
jwetzell pushed a commit to jwetzell/bruno that referenced this pull request Aug 2, 2024
* feat: path parameters (usebruno#484)

* add path parameters on bruno-app

* add path parameters on bruno-cli

* fix bruno-schema testing

* fix generate request code not replace path parameter value

---------

Co-authored-by: game5413 <febryanph10@gmail.com>
Co-authored-by: Anoop M D <anoop.md1421@gmail.com>

* feat: Refactor request parameter handling

- Update prepare-request.js to filter and rename 'paths' to 'params' with type 'path'
- Remove 'paths' from export.js and interpolate-vars.js
- Update bru.js to use 'params' instead of 'path'
- Update requestSchema in index.js to use 'keyValueWithTypeSchema' for 'params'

Co-authored-by: game5413 <febryanph10@gmail.com>
Co-authored-by: Anoop M D <anoop.md1421@gmail.com>

* feat: Refactor request parameter handling

* refactor: changes form the review

* refactor: Refactor transformItemsInCollection handling

* refactor: Refactor improved export/import functionalities

* refactor: Remove console.log statement in bruToJson.js

---------

Co-authored-by: game5413 <37659721+game5413@users.noreply.github.com>
Co-authored-by: game5413 <febryanph10@gmail.com>
Co-authored-by: Anoop M D <anoop.md1421@gmail.com>
jwetzell pushed a commit to jwetzell/bruno that referenced this pull request Aug 2, 2024
* fix(usebruno#484): minor code fixes

* code fixes

* fixes for generateCode

* var change

* pr review fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.