Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

zapier convert: Replace variables in URLs (PDE-10) #208

Merged
merged 5 commits into from
Dec 25, 2017

Conversation

eliangcs
Copy link
Member

@eliangcs eliangcs commented Dec 15, 2017

Developers can put variables from in a URL, such as https://{{account}}.example.com/{{resource_name}}. This PR makes sure we replace those variables in URLs before we send requests.

Example app to try out: 83342:

zapier convert 83342 replace-vars
cd replace-vars
npm install
zapier test

Expect zapier test to fail on a TypeError for test case Full Search and Full Trigger due to a bug in core, which was addressed in zapier/zapier-platform-core#59.

@eliangcs eliangcs changed the title zapier convert: Replace variables in URLs [WIP] zapier convert: Replace variables in URLs Dec 15, 2017
@eliangcs eliangcs changed the title [WIP] zapier convert: Replace variables in URLs zapier convert: Replace variables in URLs Dec 19, 2017
@ZaneLyon ZaneLyon changed the title zapier convert: Replace variables in URLs zapier convert: Replace variables in URLs (PDE-10) Dec 21, 2017
@ZaneLyon
Copy link
Contributor

I'm testing the Jira integration, so I added a Jira task reference to the title of this PR to see if linking is working properly.

Copy link
Contributor

@xavdid xavdid left a comment

Choose a reason for hiding this comment

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

sweet, that'll be really convenient.

Only question I had was about the reason for removing the {} argument in all those calls to replaceVars? Seems like it wouldn't do anything in that function while empty (which could be why you're removing it)

@eliangcs
Copy link
Member Author

@xavdid yeah, you guess it right. You get the same result whether you pass a {} or anundefined to replaceVars, so I removed it to make the code cleaner.

@eliangcs eliangcs merged commit 458bfb0 into master Dec 25, 2017
@eliangcs eliangcs deleted the convert-replace-vars branch December 25, 2017 05:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants