Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

Move trigger construction into triggers/ directory #14

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zimeg
Copy link
Member

@zimeg zimeg commented Feb 10, 2023

Type of change

  • New sample
  • New feature
  • Bug fix
  • Documentation

Summary

This PR follows from this conversation and relocates trigger definition logic into the triggers/ directory. This approach brings a few questions since this is a trigger created at runtime with dynamic inputs, while definitions in this directory are often static or created via the CLI.

This is only one approach for making this change. An alternative might move the entire creation process (including the API call) into the trigger definition file. Another may use a complete definition with blank inputs in place of the dynamic channel value, and populate these values from the custom function. I'm sure there are other ways to dice this too!

Open questions and thoughts

  • Overall, I do agree that it makes sense to keep trigger definitions in this directory. Wondering if this might expand to other trigger logic, such as the findUserJoinedChannelTrigger, which is unique to the SendWelcomeMessage workflow's event trigger in this app?
  • This trigger definition has dynamic inputs and therefore cannot be created using the CLI. Locating the definition in this directory may result in confused creations from the auto-creation prompt when first installing an app to a workspace.
  • A previous example of defining a trigger in the triggers/ directory that is also created at runtime can be found in the deno-simple-survey repo (trigger definition, trigger creation).
    • This repo also has examples of creating triggers by definition in functions here and here. These triggers also require dynamic inputs. Whatever decision is landed on here should probably be applied to this sample as well.
  • Is there another approach of defining the trigger then populating the channel_id values that might be better?

Requirements

  • I’ve checked my submission against the Samples Checklist to ensure it complies with all standards
  • I have ensured the changes I am contributing align with existing patterns and have tested and linted my code
  • I've read and agree to the Code of Conduct

@zimeg zimeg added the question Further information is requested label Feb 10, 2023
@zimeg zimeg requested a review from misscoded February 10, 2023 19:07
@zimeg zimeg self-assigned this Feb 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant