I think we're requiring hook payloads to contain an id

[codekirei]

AFAICT we're doing the trigger has id check on hook trigger data. Is this intentional? Makes sense for polling triggers, but seems odd that we're also running it against hooks.

https://github.com/zapier/zapier-platform-core/blob/master/src/checks/trigger-has-id.js

[bcooksey]

Yup, we are requiring that. We have this shouldRun, but for the id check, this is simply looking to see if it's a trigger.

We can discuss if this is a bug or a reasonable constraint.

[codekirei]

Cool, thanks for confirming! As for whether it's a reasonable constraint, what was the original inspiration for this check? I imagine we want to make sure polling trigger data includes an id for deduplication, but that doesn't apply for hooks.

Is there another reason we're wanting to ensure trigger data contains an id? If not, I'd be in favor of restricting this restraint to just polling triggers.

If we do decide to stick with this as-is, we'll probably want to be a bit more explicit about this requirement in our docs — I'm happy to make the that update if that's the consensus.

[bryanhelmig]

I'd be in favor of restricting this restraint to just polling triggers.

That sounds reasonable!

[xavdid]

+1 to just polling triggers to semi-match the v2 setup

[bcooksey]

In that case, we can schedule a fix

[xavdid]

Sweet. I was going to grab this and #25, since they seem very related