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

Fix default file hydrator #118

Closed
wants to merge 1 commit into from
Closed

Conversation

eliangcs
Copy link
Member

@eliangcs eliangcs commented Oct 10, 2018

Background

Default File Hydrator is a function that we inject to the app definition at runtime. It was added in PR #112 to power the newly-introduced z.dehydrateFile method. When a dev calls z.dehydrateFile(url, request, meta), they're doing z.dehydrateFile(hydrators.zapierDefaultHydrator, { url, request, meta }) essentially.

What This PR Fixes

The request and meta argument in z.dehydrateFile(url, request, meta) are optional. When a dev omits request argument, i.e., z.dehydrateFile(url), we should include the user's auth info when we make the request to url. On the other hand, if request argument is provided, we should not include the user's auth info. This is to make the behavior consistent with the Web Builder as described in this doc.

There's a doc change in PR zapier/zapier-platform-cli#360 you may want to take a look at: zapier/zapier-platform-cli@ff79a04.

@eliangcs eliangcs requested a review from xavdid October 10, 2018 14:39
@bcooksey
Copy link
Contributor

I realize that we need the (url, request, meta) for WB apps we are going to port over. Do we want to promote that pattern for fresh apps though? It seems we make our lives easier if we say the only proper form is (func, inputData). We could then have the LegacyScriptingRun add it's own definition of z.dehydrateFile() that supports the 3 argument version.

Thoughts @xavdid @eliangcs?

@xavdid
Copy link
Contributor

xavdid commented Oct 10, 2018

Yeah, i like that. we should try to keep them as separate as possible; CLI do things the best way for it and WB can adapt as needed.

@eliangcs
Copy link
Member Author

@bcooksey that makes sense. The code is getting complicated here. I'll push another version that movie all of these to legacy-scripting-runner. Closing this PR.

@eliangcs eliangcs closed this Oct 11, 2018
@eliangcs eliangcs deleted the fix-default-file-hydrator branch October 11, 2018 06:31
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