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

[fix] function execution events accept interfaces not strings #1357

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ProjectBarks
Copy link

@ProjectBarks ProjectBarks commented Dec 4, 2024

API changes [BREAKING]

In the initial implementation of this change, we assumed that the input types would always be map[string]string. However, Slack can send inputs with various types, not just strings. This update modifies the mapping to use an interface{} to handle these different input types. Additionally, the test has been updated to include an example of a real payload received from Slack to ensure proper handling of these cases.

Example
{
  "event": {
    "type": "function_executed",
    "function": {
      "id": "Fn07TRTH0EKX",
      "callback_id": "d70234",
      "title": "Test",
      "description": "Test.",
      "type": "app",
      "input_parameters": [
        {
          "type": "slack#/types/message_context",
          "name": "message_context",
          "description": "Test",
          "title": "Mesage Context",
          "is_required": true
        }
      ]
    },
    "inputs": {
      "message_context": {
        "channel_id": "C07H1ER7U66",
        "message_ts": "1733331835.871019"
      },
    },
    "function_execution_id": "Fx08387NLCNT",
    "workflow_execution_id": "Wx084BHAGCMN",
    "event_ts": "1733331846.861810"
  }
}

@calebmckay
Copy link
Contributor

Code change seems simple enough.

I updated one of the automated tests to test a variety of values for the input, including the message_context type you provide in your example. You may want to pull the changes into your PR. b005e63

@ProjectBarks
Copy link
Author

ProjectBarks commented Dec 12, 2024

Oh I actually had cases to cover this but I guess I forgot to push. I'll incorporate b005e63 as well. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants