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

Environment variables in collections #851

Conversation

sboulema
Copy link
Contributor

@sboulema sboulema commented May 7, 2020

Fixes saving a request when using environment variables.

url and path variables get overwritten with replaced values, so before saving we reconstruct them using the original uri variable.

This PR intends to fix #642.

@TravisBuddy
Copy link

Hey @sboulema,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: f8f33770-9094-11ea-931a-79bfa81bc315

@AndrewBastin AndrewBastin changed the title Environment variables in collections #642 Environment variables in collections May 7, 2020
@TravisBuddy
Copy link

Hey @sboulema,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 60ed2310-9101-11ea-ba10-7b1b65697aa0

@liyasthomas liyasthomas self-requested a review May 8, 2020 15:13
@TravisBuddy
Copy link

Hey @sboulema,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 16e79940-9141-11ea-ba10-7b1b65697aa0

@liyasthomas
Copy link
Member

liyasthomas commented May 8, 2020

Hi @sboulema , thanks for the quick PR, I like the simplicity of the implementation. But there's an issue. While restoring a request from collections, the env variables are still translated to it's evn value. This shouldn't be happening. When we reuse a saved collection request - it should restore URL with saved env quotation and not its evn value.

To reproduce: goto deploy preview: https://deploy-preview-851--postwoman.netlify.app/

1: create and save a request with env variables to collection.

image

2: click saved request from collection to restore it.

image

As you can notice, when we restore the request - the env variables have been replace with env values. The expected behaviour is: env variable should be maintained.

I think we can store the env variable as it is (without converting to env values) while saving to collections. What do you say about that? I wanna hear from you.

@TravisBuddy
Copy link

Hey @sboulema,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: d86eaa30-916f-11ea-ba10-7b1b65697aa0

@sboulema
Copy link
Contributor Author

sboulema commented May 8, 2020

Thanks for the very clear reproducing steps!
Helped in finding a couple of more cases were the URL is geting saved wrongly.

Should be ok now :)

And yes I completely agree, it should always save non-replaced version of the uri and only use replaced version when doing the actual request and showing in the history list.

@liyasthomas liyasthomas merged commit 77471a5 into hoppscotch:master May 9, 2020
@sboulema sboulema deleted the feature/environment-variables-in-collections-#642 branch May 10, 2020 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Environment variables in collections
3 participants