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

Update triggers #24

Merged
merged 2 commits into from
Sep 28, 2023
Merged

Conversation

tomas-zijdemans-vipps
Copy link
Contributor

Type of change (place an x in the [ ] that applies)

  • New sample
  • [x ] New feature
  • Bug fix
  • Documentation

Summary

Updating the triggers, similar to how the other samples have been updated, to use the new available types

Requirements (place an x in each [ ] that applies)

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

@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @tomas-zijdemans-vipps to sign the Salesforce Inc. Contributor License Agreement.

@tomas-zijdemans-vipps
Copy link
Contributor Author

I realised now that TriggerContextData.Shortcut.interactivity and TriggerContextData.Shortcut.channel_id are not ideal when the type is TriggerTypes.Scheduled. It works, but it's not pretty. The better fix would be to fix the internal TriggerTypes.Scheduled type

@zimeg zimeg added the enhancement New feature or request label Sep 28, 2023
Copy link
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

👋 Hey @tomas-zijdemans-vipps! Appreciate this change - always a fan of stronger types and all looks good to me so we'll merge it as is!

Re: the mismatched TriggerContextData.Shortcut and TriggerTypes.Scheduled values, I agree that this isn't ideal but this trigger type actually isn't supposed to have these values! This is because a scheduled trigger won't have this context when being tripped. Currently working on a change to update this in the sample since the SDK typing is already correct here, and it's pretty cool that this strange typing is raising this as a problem!

@zimeg zimeg merged commit edf57c9 into slack-samples:main Sep 28, 2023
1 check passed
@zimeg zimeg mentioned this pull request Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla:signed enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants