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: add app event filter type to functions [EXT-5089] #566

Merged
merged 9 commits into from
Mar 27, 2024

Conversation

whitelisab
Copy link
Contributor

This PR adds the new appevent.filter event type to the function handler for app functions. This then can be imported into our example apps that utilize functions.

@whitelisab whitelisab requested a review from a team as a code owner March 14, 2024 17:05
entityProps: T
entityAction: string // 'create' | 'publish' etc etc.
}
// TODO: use generic to DRY this up?
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get rid of this comment now?

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like it! but also we need a TODO to make this exhaustive of all of the other entity types/actions that an app event subscription can receive (e.g. all of these that apply to AppEvents https://www.contentful.com/developers/docs/webhooks/content-events/)

eligible app event topics/actions
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this comment // TODO: use generic to DRY this up? and updated a comment to reflect that we need to update the app event subscription entities and topics/actions // TODO: add all of the other app event subscription entities and topics/actions

@ghepting ghepting marked this pull request as draft March 14, 2024 17:29

export type AppEventFilterResponse = {
result: boolean
errors?: readonly Record<string, any>[]
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the intent of this property?

Copy link
Contributor

Choose a reason for hiding this comment

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

we were following suit with delivery function response shape in case there were errors to bubble up, but we're not 100% sure if we need this yet to be honest. hence this being a draft still as we connect the full e2e dots together 😄

Copy link
Contributor

@ghepting ghepting Mar 15, 2024

Choose a reason for hiding this comment

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

i think i was imaging something like:

  • function expects entry Foo to have "slug" field, but it doesn't
  • so it returns false as the result value causing the app event to be filtered and not sent to the final destination & includes error details something like: "missingExpectedField": "No "slug" field to validate on entry. Field not present in ContentType "blogPost" or whatever which could be useful elsewhere like the logs? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

The example makes sense 👍🏻 The question is who takes care of logging that, is that the filter function itself or do you want to put that concern on the system invoking the function?

For context: The errors prop in the graphql.query response comes from the fact that we simply follow the GQL spec and specifically this part:

If a field error is raised, execution attempts to continue and a partial result is produced (see Handling Field Errors). The data entry in the response must be present. The errors entry should include all raised field errors.

@mgoudy91 mgoudy91 changed the title feat: add app event filter type to functions [EXT-5019] feat: add app event filter type to functions [EXT-5089] Mar 18, 2024
@david-shibley-contentful david-shibley-contentful marked this pull request as ready for review March 27, 2024 17:35
src/requests/typings/function.ts Outdated Show resolved Hide resolved
src/requests/typings/function.ts Outdated Show resolved Hide resolved
Co-authored-by: Tyler Collins <104802020+t-col@users.noreply.github.com>
Co-authored-by: Tyler Collins <104802020+t-col@users.noreply.github.com>
@david-shibley-contentful david-shibley-contentful merged commit f8ea2cb into master Mar 27, 2024
7 checks passed
@david-shibley-contentful david-shibley-contentful deleted the feat/cca-cli-app-event-filter branch March 27, 2024 20:50
contentful-automation bot added a commit that referenced this pull request Mar 27, 2024
# [3.3.0](v3.2.1...v3.3.0) (2024-03-27)

### Features

* add app event filter type to functions [EXT-5089] ([#566](#566)) ([f8ea2cb](f8ea2cb))
@contentful-automation
Copy link
Contributor

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants