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 email shows as unverified when no email server is configured #3613

Merged

Conversation

rauchy
Copy link
Contributor

@rauchy rauchy commented Mar 20, 2019

What type of PR is this? (check all applicable)

  • Refactor
  • Bug Fix

Description

Related Tickets & Documents

Fixes #3608

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

@rauchy rauchy requested a review from arikfr March 20, 2019 11:18
@ghost ghost assigned rauchy Mar 20, 2019
@ghost ghost added the in progress label Mar 20, 2019
@@ -141,6 +141,11 @@ def all_settings():
MAIL_MAX_EMAILS = os.environ.get('REDASH_MAIL_MAX_EMAILS', None)
MAIL_ASCII_ATTACHMENTS = parse_boolean(os.environ.get('REDASH_MAIL_ASCII_ATTACHMENTS', 'false'))


def email_server_is_configured():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figured it's best to hide this instead of relying on MAIL_DEFAULT_SENDER in different places in the app, in case we ever want to change the criteria.

@arikfr arikfr merged commit 49ffaae into master Mar 27, 2019
@arikfr arikfr deleted the fix-email-shows-as-unverified-when-no-server-configured branch March 27, 2019 15:57
@arikfr
Copy link
Member

arikfr commented Mar 27, 2019

👍

harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
…redash#3613)

* check that e-mail server is configured before marking the email address
as not verified and sending out a verification e-mail

* use helper method in `invite_user`

* move email_server_configured helper to settings

* add test to verify that email addresses arent marked as unverified if
there's no e-mail server to verify them

* simplify a couple of tests with patch

* combine conditions into single variable

* Booleans, gotta love 'em
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.

3 participants