-
Notifications
You must be signed in to change notification settings - Fork 1
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: Adding output fields for dataset items #51
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the pull request and linked issue has estimate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the pull request and linked issue has estimate
@@ -130,7 +130,7 @@ const enrichActorRun = async (z, run, storeKeysToInclude = []) => { | |||
}; | |||
|
|||
// Process to subscribe to Apify webhook | |||
const subscribeWebkook = async (z, bundle, condition) => { | |||
const subscribeWebhook = async (z, bundle, condition) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fixing typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 6 out of 20 changed files in this pull request and generated no suggestions.
Files not reviewed (14)
- package.json: Language not supported
- index.js: Evaluated as low risk
- test/zapier_helpers.js: Evaluated as low risk
- CHANGELOG.md: Evaluated as low risk
- test/helpers/index.js: Evaluated as low risk
- test/creates/actor_run.js: Evaluated as low risk
- src/zapier_helpers.js: Evaluated as low risk
- src/apify_helpers.js: Evaluated as low risk
- src/triggers/actor_run_finished.js: Evaluated as low risk
- src/triggers/task_run_finished.js: Evaluated as low risk
- src/searches/task_last_run.js: Evaluated as low risk
- src/searches/actor_last_run.js: Evaluated as low risk
- src/creates/actor_run.js: Evaluated as low risk
- src/creates/task_run.js: Evaluated as low risk
Comments skipped due to low confidence (2)
src/output_fields.js:27
- The function 'getDatasetItemsOutputFields' should handle the case where 'items' is empty more gracefully. Consider adding a check before merging items.
if (items.length === 0) return [];
test/creates/task_run.js:47
- The change from 'expect(testResult).to.have.all.keys' to 'expect(testResult).to.have.any.keys' reduces the strictness of the test. This could allow for unexpected keys to be present in the result, potentially hiding issues.
expect(testResult).to.have.any.keys(Object.keys(TASK_RUN_SAMPLE).concat(['isStatusMessageTerminal', 'statusMessage']));
In this PR we are adding dynamic outputFields for the action which returns datset items. This requires parsing JavaScript plain object into FieldsShema, the definition of schema is here, this is also part of this PR.
Why we need this:
When user connect Apify into some Zapier flow the output data from Actor run dataset items cannot be easily mapped into next flow, the reason was that we are missing the dataset items into outputFields. See the problem users face - here is reported issue.
By adding dataset items into outputFields we solve this and the user can easily map the dataset data between Zapier flows.
How to test on Zapier platform
The new version is in preview mode. You need to be part of the Apify developers group on Zapier; I added you.
![image](https://private-user-images.githubusercontent.com/11837643/389196929-1257685d-3114-46aa-9cf0-9fb020c4f7fe.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkzOTQzODcsIm5iZiI6MTczOTM5NDA4NywicGF0aCI6Ii8xMTgzNzY0My8zODkxOTY5MjktMTI1NzY4NWQtMzExNC00NmFhLTljZjAtOWZiMDIwYzRmN2ZlLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTIlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjEyVDIxMDEyN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTUyOTQ2ZjkzOTYzY2Q2NWI1OWY0OTJiYmJkMjc4M2YxYzI3YWQ0ZTdhZDg3MzE3M2VmYjhjMTNlMmJmYmY3NDkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.lFK5KIAvx2o22uxAKUvfWZxS2q6izb68tq3yOfFs1yI)
Then you will see the Apify version when you want to create the Zap, the tested version is 3.2.0.
fixes #48