Skip to content
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 direct_messages_max_per_day set to nil #2100

Conversation

deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Oct 23, 2017

When set to nil, it should mean not zero, but "infinite".

Where

None.

What

  • Whats the objective of this changes ?

The objective is that there's a way to have unlimited direct messages per day, and also that messages can be sent by default on a development setup, since it currently gives a "max messages per day reached" error that prevents every message from being sent.

How

  • How you implemented/achieved the objective ?

I checked for the existance of the setting and skip validation in that case. This way we skip the automatic nil.to_i (== 0) conversion leading to the current behavior.

Screenshots

None.

Test

  • Is manual test needed or you just increased/fixed coverage?

I added coverage.

Deployment

  • Any details to remember when this feature is deployed?

I don't think so.

Warnings

  • Some caveats or important things to notice?

Nope. Well, maybe yes. If some installation is intentionally using this (buggy, in my opinion) behavior, they'll have to start being explicit about it and set Setting['direct_max_messages_per_day'] = 0.

When set to nil, it should mean not zero, but "infinite".
@deivid-rodriguez
Copy link
Contributor Author

I'm assuming the failure is unrelated, let me know if I'm wrong though.

@voodoorai2000
Copy link
Member

voodoorai2000 commented Nov 9, 2017

Thank you so much @deivid-rodriguez!
Sorry for the delay we are a little overwhelmed 🙏 🙇

@voodoorai2000 voodoorai2000 merged commit 0e5c8d6 into consuldemocracy:master Nov 9, 2017
@deivid-rodriguez deivid-rodriguez deleted the fix/direct_messages_max_per_day branch November 10, 2017 11:35
@deivid-rodriguez
Copy link
Contributor Author

No problem, thanks to you! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants