-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: Polls limiting duration at 7 days #9871
Conversation
if (duration.total_seconds() / 3600) > 168: # As the duration may exceed little milliseconds then we fix it | ||
duration = datetime.timedelta(days=7) | ||
|
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.
Why are we removing this clamp instead of altering it to meet the new criteria?
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 think its better to remove check, since unexpiring polls might come out later
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.
...and upon such time we would remove said clamp, we shouldn't be doing it for features we hope discord may release.
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.
The check is removed so polls aren't capped at 7 days when obtained via API or gateway, in my opinion updating this every time Discord updates the polls maximum duration would not be right.
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.
If that is the argument now then I wonder why it was implemented and accepted in the first place.
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.
It's unlikely discord ever adds an uncapped poll. The feature doesn't make sense. The point of a poll is getting an answer from a group of people, if it never ends, there's no answer ever.
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.
It's fine to remove the check and let the server handle it, that's what the library usually does anyway except within discord.ui
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.
It's unlikely discord ever adds an uncapped poll. The feature doesn't make sense. The point of a poll is getting an answer from a group of people, if it never ends, there's no answer ever.
Discord still might add this per discord/discord-api-docs#6746 (comment)
Damn this thing closed lol, sorry about that gonna reopen it |
Summary
So Discord recently allowed polls to be 32 days long so the check in
Poll._from_data
that "fixes" the poll duration at 7 days should be removed.This also makes sure that if Discord changes the maximum duration for polls again there won't be any problems.
Checklist