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(channel-vonage): add support for file reception #4914

Merged
merged 25 commits into from May 6, 2021

Conversation

laurentlp
Copy link
Contributor

This PR adds support for file reception on channel-vonage

Copy link
Contributor

@samuelmasse samuelmasse left a comment

Choose a reason for hiding this comment

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

Tested and working 👍

I think this feature needs a better explanation of how it works. I didn't realize the payload.url property was overwritten by the new local url. I also didn't realize I needed to supply {{{event.payload.url}}} as an argument for it to work. I think by default the action should check if payload.url is set and use it without needing to set the type parameter.

Also, I think the type parameter is only doing half of the job. If you decide to pick another property in your payload to pass as a parameter, you still have the new url set to the payload.url property, which isn't ideal. What I would like is to give a path (for example payload.url) and then have the action use _.get(event, path) and _.set(event, path) to get the old url and replace it.

modules/channel-vonage/src/actions/storeFileLocally.js Outdated Show resolved Hide resolved
modules/channel-vonage/src/actions/storeFileLocally.js Outdated Show resolved Hide resolved
modules/channel-vonage/src/backend/client.ts Outdated Show resolved Hide resolved
modules/channel-vonage/src/actions/storeFileLocally.js Outdated Show resolved Hide resolved
modules/channel-vonage/src/backend/client.ts Outdated Show resolved Hide resolved
modules/channel-vonage/src/backend/client.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@samuelmasse samuelmasse left a comment

Choose a reason for hiding this comment

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

Approved and working for anything under 10mb. We'll need to find a fix for that random 10mb limit in actions

@laurentlp laurentlp merged commit a0384ce into master May 6, 2021
@laurentlp laurentlp deleted the llp_channel_vonage_file_reception branch May 6, 2021 21:35
@EFF EFF mentioned this pull request May 14, 2021
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.

None yet

2 participants