Skip to content

[Notifier][Slack] Error trown when having more than 10 fields specified #36346 #36347

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
Jun 25, 2020

Conversation

birkof
Copy link
Contributor

@birkof birkof commented Apr 4, 2020

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

As written in docs of Section block, for fields we have to had maximum 10 items specified.

@birkof birkof changed the title Maximum number of items is 10 [Notifier][Slack] Error trown when having more than 10 fields specified #36346 Apr 4, 2020
@fabpot
Copy link
Member

fabpot commented Jun 24, 2020

@birkof Have you seen my comment? Do you agree?

@birkof birkof force-pushed the notifier/limitSlackSectionFields branch from f898a1c to 2ea2b64 Compare June 24, 2020 23:09
@fabpot fabpot force-pushed the notifier/limitSlackSectionFields branch from 2ea2b64 to 68b2490 Compare June 25, 2020 07:24
@fabpot
Copy link
Member

fabpot commented Jun 25, 2020

Thank you @birkof.

@fabpot fabpot merged commit 213c6ab into symfony:master Jun 25, 2020
@@ -44,6 +44,11 @@ public function field(string $text, bool $markdown = true): self
'text' => $text,
];

// Maximum number of items is 10
if (10 <= \count($this->options['fields'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should be <, with <= you throw not only for count > 10 but also for count == 10

Copy link
Member

Choose a reason for hiding this comment

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

Good catch, fixed in 7bac792

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@guilliamxavier you're right! I have tested more over here and you're right!

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.

[Notifier][Slack] Error trown when having more than 10 fields specified
5 participants