feat: Add input field to select finished run state for triggers #56

matyascimbulka · 2025-04-28T07:57:42Z

Links to issue #16.

This change adds a multiple choice dropdown input field for actor / task run triggers which allows selection of specific terminal state for the trigger. As describe in #16 this change will allow free users to filter the runs based on the terminal status.

This solution has a few issues that mostly stem from imperfections in Zapier input UI and it's currently possible to select one state more then once and even enter a custom value to the dropdown. However, both cases are handled in the code and result in creation of correct web hooks.

For reference here is a screenshot of the resulting input UI on the Zapier platform:

Copilot

Pull Request Overview

This PR adds a multiple choice dropdown input field for actor/task run triggers that allows filtering based on terminal run states. The key changes include updating test bundles to include a new "states" field, adding a new field in the trigger definitions with choices from ACTOR_RUN_TERMINAL_STATES, and modifying the webhook subscription helper to dynamically compute event types based on the selected states.

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/triggers/task_run_finished.js	Updated test input to include "states" using Object.keys(ACTOR_RUN_TERMINAL_STATES).
test/triggers/actor_run_finished.js	Updated test input to include "states" using Object.keys(ACTOR_RUN_TERMINAL_STATES).
src/triggers/task_run_finished.js	Added new "states" field (dropdown) for task run triggers.
src/triggers/actor_run_finished.js	Added new "states" field (dropdown) for actor run triggers.
src/consts.js	Defined ACTOR_RUN_TERMINAL_STATES with terminal state mappings.
src/apify_helpers.js	Modified subscribeWebhook to compute eventTypes from filtered states.
CHANGELOG.md	Documented the new feature in the changelog.

Files not reviewed (1)

package.json: Language not supported

Comments suppressed due to low confidence (2)

src/triggers/task_run_finished.js:48

[nitpick] The helpText for the 'states' field in the task run trigger still refers to 'actor'. Consider updating the text to clarify that it applies to task runs or to use terminology consistent with the trigger's context.

{ label: 'States', helpText: 'Please select the terminal states of the actor.', key: 'states', required: true, list: true, choices: ACTOR_RUN_TERMINAL_STATES, },

src/apify_helpers.js:135

[nitpick] Consider adding test cases for scenarios with duplicate states and for custom, potentially invalid state values in the input to ensure that subscribeWebhook handles these cases correctly.

const states = Array.from(new Set(bundle.inputData.states));

drobnikj

Good start :)

On top of my comments please:

Update performList to use statuses for Actor/task see https://github.com/apify/apify-zapier-integration/pull/56/files#diff-d4fdc89364a70fd9a9f2a2b27eb9757764de197b2270aaf351f83bbe1894b3ffR58
Add tests cover new code

drobnikj · 2025-04-29T11:56:56Z

src/consts.js

@@ -270,6 +270,13 @@ const ALLOWED_MEMORY_MBYTES_LIST = Array.from(

 const DEFAULT_ACTOR_MEMORY_MBYTES = 2048;

+const ACTOR_RUN_TERMINAL_STATES = {


As keys please use enums from apify consts
https://github.com/apify/apify-shared-js/blob/master/packages/consts/src/consts.ts#L30

drobnikj · 2025-04-29T11:58:39Z

src/triggers/actor_run_finished.js

+                label: 'States',
+                helpText: 'Please select the terminal states of the actor run.',
+                key: 'states',
+                required: true,


Please use the default values the same as in the previous version of the WEBHOOK_EVENT_TYPE_GROUPS.ACTOR_RUN_TERMINAL

Make the field optional, and if the user did not fill any state use WEBHOOK_EVENT_TYPE_GROUPS.ACTOR_RUN_TERMINAL as fallback

If we do it like this we did not break, as we keep current behaviour

Comment this behaviour in help text mainly the fact that if no value is provided we use all terminal state as fallback.

drobnikj · 2025-04-29T12:06:58Z

src/triggers/actor_run_finished.js

@@ -40,6 +40,14 @@ module.exports = {
                required: true,
                dynamic: 'actors.id.name',
            },
+            {
+                label: 'States',


It is status not state :)

Let's update it everywhere

drobnikj · 2025-04-29T12:07:56Z

src/triggers/actor_run_finished.js

@@ -40,6 +40,14 @@ module.exports = {
                required: true,
                dynamic: 'actors.id.name',
            },
+            {
+                label: 'States',
+                helpText: 'Please select the terminal states of the actor run.',


Actor always with a capital, please check writing guide.

And the helpText need some love, let's consult this with some copy writer where we will be done with code changes.

matyascimbulka · 2025-04-30T08:23:30Z

@drobnikj Thank you for the review. I believe that I've address most of your comments from the review. The only thing that I'm not sure how to update the performList function. The main issue is that the Apify API endpoint which returns list of runs only accepts single status for filtering. The only way to get runs just for the selected statuses is to make multiple request to our API.

Would this be a good solution or do you have any other ideas about this?

drobnikj · 2025-04-30T12:48:36Z

Would this be a good solution or do you have any other ideas about this?

@matyascimbulka There is already an issue on the platform to add support for this. But in the meantime let's make multiple requests, it is not nice workaround but it can work.

matyascimbulka · 2025-05-05T13:55:27Z

@drobnikj I think I have managed to address all of your comments.

As far as the performList functionality goes I have ended up doing it the same way as before. Which is fetch all runs and filter them based on status. I haven't written any tests for this specifically because I'm already having trouble with failing tests on GitHub. Fixing the tests should be a different issue.

Copilot

Pull Request Overview

This PR adds a multiple‐choice dropdown input field to allow filtering of actor/task run triggers by terminal state. Key changes include new test cases to verify webhook creation with various status combinations, updates to the task and actor trigger implementations to use the selected statuses, and the addition of a new constant for terminal statuses.

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/triggers/create_webhook_with_status.js	New tests verifying webhook creation for actor/task triggers with terminal status filtering
src/triggers/task_run_finished.js	Updated filtering logic and added the statuses field for task triggers
src/triggers/actor_run_finished.js	Updated filtering logic and added the statuses field for actor triggers
src/consts.js	Added constant ACTOR_RUN_TERMINAL_STATUSES for terminal statuses
src/apify_helpers.js	Introduced getActorStatusesFromBundle and updated subscribeWebhook to support status mapping
CHANGELOG.md	Updated changelog to document the new feature

Files not reviewed (1)

package.json: Language not supported

Copilot · 2025-05-07T11:39:21Z

test/triggers/create_webhook_with_status.js

+            .post('/v2/webhooks', (payload) => {
+                expect(payload).to.have.property('requestUrl', requestUrl);
+                expect(payload).to.have.property('condition').that.has.property('actorId', testActorId);
+                expect(payload).to.have.property('eventTypes')


The test expects WEBHOOK_EVENT_TYPES.ACTOR_RUN_TIMED_OUT to be both included (line 39) and excluded (line 41) from the eventTypes array. Please revisit the intended behavior and update the assertions accordingly.

Copilot · 2025-05-07T11:39:22Z

src/apify_helpers.js

 // Process to subscribe to Apify webhook
 const subscribeWebhook = async (z, bundle, condition) => {
+    const statuses = getActorStatusesFromBundle(bundle);
+
+    const eventTypes = statuses.map((status) => status.replace('-', '_')).map((status) => {


Mapping the statuses to event types may produce undefined values when no match is found. Consider filtering out these undefined values before using the array in webhookOpts.

drobnikj

Just last bits

drobnikj · 2025-05-07T11:39:43Z

src/triggers/task_run_finished.js

@@ -45,6 +46,14 @@ module.exports = {
                required: true,
                dynamic: 'tasks.id.name',
            },
+            {
+                label: 'Statuses',
+                helpText: 'Please select the terminal states of the task run. If no status is selected, all terminal statuses will be used.',


Suggested change

helpText: 'Please select the terminal states of the task run. If no status is selected, all terminal statuses will be used.',

helpText: 'Select one or more terminal statuses for the run. If none are selected, all terminal statuses will be included by default.',

drobnikj · 2025-05-07T12:15:06Z

src/apify_helpers.js

 // Process to subscribe to Apify webhook
 const subscribeWebhook = async (z, bundle, condition) => {
+    const statuses = getActorStatusesFromBundle(bundle);
+
+    const eventTypes = statuses.map((status) => status.replace('-', '_')).map((status) => {


Wow, I have no idea that we designed it that bad :D mixing - and _.

drobnikj · 2025-05-07T12:18:58Z

src/apify_helpers.js

+
+    const eventTypes = statuses.map((status) => status.replace('-', '_')).map((status) => {
+        // eslint-disable-next-line no-restricted-syntax
+        for (const eventType of Object.values(WEBHOOK_EVENT_TYPES)) {


This is not nice you need to loop every time.
What about doing a simple key-value object or Map where you map status to event time and use this?
You would not need to replace '-', '_'.

drobnikj · 2025-05-07T12:30:40Z

src/triggers/actor_run_finished.js

@@ -40,6 +41,14 @@ module.exports = {
                required: true,
                dynamic: 'actors.id.name',
            },
+            {
+                label: 'Statuses',
+                helpText: 'Please select the terminal statuses of the Actor run. If no status is selected, all terminal statuses will be used.',


Suggested change

helpText: 'Please select the terminal statuses of the Actor run. If no status is selected, all terminal statuses will be used.',

helpText: 'Select one or more terminal statuses for the run. If none are selected, all terminal statuses will be included by default.',

matyascimbulka · 2025-05-07T13:37:19Z

Thank you for the suggestions. I have implemented them and its ready for review again. The test version on Zapier is 4.0.12.

Copilot

Pull Request Overview

This PR adds a multiple choice dropdown for selecting terminal run statuses in actor/task triggers to enable free users to filter runs by terminal state. The key changes include updating trigger logic to filter runs based on selected statuses, adding new constants for terminal statuses/event types, and updating tests to validate the new behavior.

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/triggers/create_webhook_with_status.js	Added new test cases for multiple status selections and handling of invalid status values.
src/triggers/task_run_finished.js	Updated run filtering logic to consider multiple terminal statuses from bundle input.
src/triggers/actor_run_finished.js	Updated run filtering logic and added a new input field for selecting terminal statuses.
src/consts.js	Introduced constants for ACTOR_RUN_TERMINAL_STATUSES and ACTOR_RUN_TERMINAL_EVENT_TYPES.
src/apify_helpers.js	Added a helper function (getActorStatusesFromBundle) to deduplicate and validate status values and map them correctly.
CHANGELOG.md	Documented the new feature and its behavior.

Files not reviewed (1)

package.json: Language not supported

Comments suppressed due to low confidence (2)

test/triggers/create_webhook_with_status.js:121

[nitpick] The test description has a duplicate 'with'; consider renaming it to 'create webhook with wrong values' for clarity.

it('create webhook with with wrong values', async () => {

src/apify_helpers.js:140

To ensure consistency with the new constants introduced in src/consts.js, consider using ACTOR_RUN_TERMINAL_STATUSES instead of ACTOR_JOB_TERMINAL_STATUSES as the fallback.

return filteredStatuses.length > 0 ? filteredStatuses : ACTOR_JOB_TERMINAL_STATUSES;

drobnikj

just one nit, but approving anyway.
Let's add someone from content team to check helpText etc.

src/triggers/task_run_finished.js

drobnikj · 2025-05-09T09:32:59Z

I added @TheoVasilis as he checked some content in the Zapier app in the past.
Can you please check the content here? Thank you!

…ctorRuns

drobnikj · 2025-05-12T08:27:09Z

Changing to @davidjohnbarton as Theo is off.

feat: Add input field to select finished run state for triggers

3fdf66b

matyascimbulka requested review from drobnikj and Copilot April 28, 2025 07:57

Copilot AI reviewed Apr 28, 2025

View reviewed changes

matyascimbulka added 2 commits April 28, 2025 10:01

fix: Fix lint errors

d68d500

refactor: Address issues from copilot code review

b889e6d

drobnikj requested changes Apr 29, 2025

View reviewed changes

matyascimbulka added 2 commits April 30, 2025 09:59

refactor: Refactor code and add tests for PR

4522bf3

fix: Fix lint errors

1f25575

matyascimbulka added 2 commits May 5, 2025 11:21

merge: Merge master branch and resolve conflicts

af92d74

feat: performList honors statuses selected in trigger input

5e38e51

matyascimbulka requested a review from drobnikj May 5, 2025 13:55

drobnikj requested a review from Copilot May 7, 2025 11:38

Copilot AI reviewed May 7, 2025

View reviewed changes

drobnikj requested changes May 7, 2025

View reviewed changes

refactor: Implement suggestions from PR

bd6f93a

matyascimbulka requested review from drobnikj and Copilot May 7, 2025 13:37

Copilot AI reviewed May 7, 2025

View reviewed changes

fix: CHANGELOG.md (#61)

1f9bb67

drobnikj approved these changes May 9, 2025

View reviewed changes

src/triggers/task_run_finished.js Outdated Show resolved Hide resolved

drobnikj requested a review from TheoVasilis May 9, 2025 09:31

matyascimbulka added 2 commits May 9, 2025 13:54

refactor: Rename variable in task_run_finished.js -> getFallbackTashA…

d5ec9b8

…ctorRuns

merge: Fix conflicts with master

a04928f

drobnikj removed the request for review from TheoVasilis May 12, 2025 08:25

drobnikj requested a review from davidjohnbarton May 12, 2025 08:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add input field to select finished run state for triggers #56

feat: Add input field to select finished run state for triggers #56

matyascimbulka commented Apr 28, 2025

Copilot AI left a comment

drobnikj left a comment

drobnikj Apr 29, 2025

drobnikj Apr 29, 2025

drobnikj Apr 29, 2025

drobnikj Apr 29, 2025

drobnikj Apr 29, 2025

drobnikj Apr 29, 2025

drobnikj Apr 29, 2025

matyascimbulka commented Apr 30, 2025

drobnikj commented Apr 30, 2025 •

edited

Loading

matyascimbulka commented May 5, 2025

Copilot AI left a comment

Copilot AI May 7, 2025

Copilot AI May 7, 2025

drobnikj left a comment

drobnikj May 7, 2025

drobnikj May 7, 2025

drobnikj May 7, 2025

drobnikj May 7, 2025

matyascimbulka commented May 7, 2025

Copilot AI left a comment

drobnikj left a comment

drobnikj commented May 9, 2025

drobnikj commented May 12, 2025

		@@ -270,6 +270,13 @@ const ALLOWED_MEMORY_MBYTES_LIST = Array.from(

		const DEFAULT_ACTOR_MEMORY_MBYTES = 2048;

		const ACTOR_RUN_TERMINAL_STATES = {

	helpText: 'Please select the terminal states of the task run. If no status is selected, all terminal statuses will be used.',
	helpText: 'Select one or more terminal statuses for the run. If none are selected, all terminal statuses will be included by default.',

feat: Add input field to select finished run state for triggers #56

Are you sure you want to change the base?

feat: Add input field to select finished run state for triggers #56

Conversation

matyascimbulka commented Apr 28, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

drobnikj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matyascimbulka commented Apr 30, 2025

drobnikj commented Apr 30, 2025 • edited Loading

matyascimbulka commented May 5, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Copilot AI May 7, 2025

Choose a reason for hiding this comment

drobnikj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matyascimbulka commented May 7, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

drobnikj left a comment

Choose a reason for hiding this comment

drobnikj commented May 9, 2025

drobnikj commented May 12, 2025

drobnikj commented Apr 30, 2025 •

edited

Loading