-
Notifications
You must be signed in to change notification settings - Fork 796
Limit the number of asignees on a card #2153
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
Conversation
35b9fbd to
15cb1f7
Compare
jzimdars
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get why we need to do this but I'd suggest increasing the limit to the amount that provides the most protection but is large enough that almost no real user would hit it in legitimate use.
Could we make it 25, 50, 75... without compromising the protection? These kinds of limits feel arbitrary to users so my hope is that no one ever actually sees this.
That said, I made a few small tweaks to the notice display in 140399a
| </ul> | ||
|
|
||
| <div class="popup__footer" hidden data-assignment-limit-target="limitMessage"> | ||
| Maxium <%= Assignment::LIMIT %> assignees |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo here! Maximum vs Maxium
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh! Thanks for catching that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this critical failure here 52bdc8d
Helps prevent DoS attacks with #2118, but also makes rendering cards and card previews more predictable.
When 10 people are assigned:

If fewer than 10 people are assigned:
