Skip to content

[Notifier] Fix slack section block #39236

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

Merged
merged 1 commit into from
Dec 2, 2020

Conversation

norkunas
Copy link
Contributor

Q A
Branch? 5.2
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

In SlackSectionBlock fields array is not initialized directly in the constructor so when running tests and trying to add a field to it throws Undefined index: fields.

@jderusse
Copy link
Member

Could you please add a test?

@norkunas norkunas force-pushed the fix-slack-section-block branch from d67ce73 to f798e57 Compare November 30, 2020 13:20
@norkunas
Copy link
Contributor Author

Could you please add a test?

Sure, added.

@norkunas norkunas force-pushed the fix-slack-section-block branch from f798e57 to 0efb504 Compare November 30, 2020 13:21
@norkunas norkunas force-pushed the fix-slack-section-block branch from 0efb504 to 472fa3b Compare November 30, 2020 17:08
@fabpot
Copy link
Member

fabpot commented Dec 2, 2020

Thank you @norkunas.

@fabpot fabpot merged commit 4b97b16 into symfony:5.2 Dec 2, 2020
@norkunas norkunas deleted the fix-slack-section-block branch December 3, 2020 12:56
fabpot added a commit that referenced this pull request Dec 5, 2020
…k action block (malteschlueter)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Notifier] Check for maximum number of buttons in slack action block

| Q             | A
| ------------- | ---
| Branch?       | 5.x |
| Bug fix?      | no |
| New feature?  | yes |
| Deprecations? | no |
| Tickets       | - |
| License       | MIT |
| Doc PR        | - |

After having problems with the SlackSectionBlock (#39236) i thought it would be helpful to also have a check for the maximum elements in the SlackActionsBlock and a test. https://api.slack.com/reference/block-kit/blocks#actions
Edit: The actual documentation says that the maximum are 5 elements but this is outdated. The actual number is 25. The slack support confirmed that.

Can this be added to 5.2 or better to the 5.x branch?

There are also some other implementations of slack blocks like the SlackDividerBlock but they have only a constructor and no additional methods. Should we also add some tests for them even if they have no extra logic?

Commits
-------

a7936d2 [Notifier] Check for maximum number of buttons in slack action block
@fabpot fabpot mentioned this pull request Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants