Fix: Failing alert test

[ranbena]

• Bug Fix

Description

Fixes alert test failure.

The bug

When email server is configured in Redash, user creation api omits invite_link which the test was dependent on.
I assume this succeeded in the feature branch tests cause it wasn't up to date with #4173.

The fix

Added no_invite to api request and now the invite_link does appear and test continues.
Also added clear error if this prop doesn't show up in the future.

[arikfr]

I assume this succeeded in the feature branch tests cause it wasn't up to date with #4173.

This is correct, but not for the reason you think: the default setting in #4173 broke the "is email server configured" test 🤕 I will fix this.

[ranbena]

@arikfr #4226 is important but not a replacement - this commit makes the test independent of the environment setting, therefore I think a must.

[arikfr]

This commit is using a bug that will be patched in a follow up pull request. Redash should only send the invite url if there is no email server configured. If you want to make the test independent of the env we have two options: 1. Make the behavior of whether to send invite_url have its own setting so we can turn it on explicitly in E2E tests. 2. Make users creation in E2E tests not rely on this (by directly updating the db?).

…

On Tue, Oct 8, 2019, 07:22 Ran Byron ***@***.***> wrote: @arikfr <https://github.com/arikfr> #4226 <#4226> is important but not a replacement - this commit makes the test independent of the environment setting, therefore I think a must. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4225?email_source=notifications&email_token=AAAROLGCIVSEEJXLRW7GBOLQNQDJ7A5CNFSM4I6IAJJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEASUGGY#issuecomment-539312923>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAROLFKMYMGM6ASOU7BLBLQNQDJ7ANCNFSM4I6IAJJQ> .

[ranbena]

How about returning invite_link in both cases?

redash/redash/handlers/users.py

Lines 44 to 50 in 3b7efb8

	invite_url = invite_link_for_user(user)
	if settings.email_server_is_configured() and send_email:
	send_invite_email(inviter, user, invite_url, org)
	else:
	d['invite_link'] = invite_url
	return d

[arikfr]

What both cases?

…

On Tue, Oct 8, 2019, 08:12 Ran Byron ***@***.***> wrote: How about returning invite_link in both cases? https://github.com/getredash/redash/blob/3b7efb8c1fc2cb8ca16a623300062f73bbb3bbf3/redash/handlers/users.py#L44-L50 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4225?email_source=notifications&email_token=AAAROLCB4DOJPFOWASGKFETQNQJB7A5CNFSM4I6IAJJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEASXZNY#issuecomment-539327671>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAROLA5QFFJSMN6BR6L3TTQNQJB7ANCNFSM4I6IAJJQ> .

[ranbena]

If email can be sent or not.

invite_url = invite_link_for_user(user) 
if settings.email_server_is_configured() and send_email: 
  send_invite_email(inviter, user, invite_url, org) 
 
d['invite_link'] = invite_url   
return d 

[arikfr]

You missed the point: with the way things implemented right now (using an invite link validates email address), we shouldn't be sharing the invite URL if email server is enabled.

But I thought about it some more and I think that what we can do is:

1. Stop validating email address when using an invite URL and always send a validation email after creating the user/after the user signs in the first time (depends on flow).
2. Once 1 is implemented, we can always return the invite URL.