Skip to content
This repository was archived by the owner on Sep 3, 2025. It is now read-only.

Conversation

@whitdog47
Copy link
Contributor

@whitdog47 whitdog47 commented May 16, 2025

This PR fixes an issue where PlainOption in Slack blocks requires string values by explicitly converting the "value" keys in options and initial_option dictionaries to strings.

@whitdog47 whitdog47 requested review from Copilot and wssheldon May 16, 2025 21:35
@whitdog47 whitdog47 self-assigned this May 16, 2025
@whitdog47 whitdog47 added the bug Something isn't working label May 16, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where PlainOption in Slack blocks requires string values by explicitly converting the "value" keys in options and initial_option dictionaries to strings.

  • Added processing for options and initial_option in static_select_block
  • Added processing for options in multi_select_block
Comments suppressed due to low confidence (2)

src/dispatch/plugins/dispatch_slack/fields.py:268

  • [nitpick] Consider renaming the loop variable 'x' to a more descriptive name such as 'option' for clarity.
for x in options:

src/dispatch/plugins/dispatch_slack/fields.py:306

  • [nitpick] Consider renaming the loop variable 'x' to a more descriptive name such as 'option' for clarity.
for x in options:

Copy link
Contributor

@wssheldon wssheldon left a comment

Choose a reason for hiding this comment

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

we should update the type signature here to be correct and plan to address the callers incorrect usage, this masks the real issue imo

@whitdog47 whitdog47 merged commit 4a546ff into main May 16, 2025
9 checks passed
@whitdog47 whitdog47 deleted the fix/plainoption-requires-strings branch May 16, 2025 21:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants