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

feat: check if gateway is valid using image #1846

Merged
merged 1 commit into from Sep 2, 2021
Merged

feat: check if gateway is valid using image #1846

merged 1 commit into from Sep 2, 2021

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Sep 2, 2021

Closes #1844 by using the same code we use in Companion (check mentioned issue).

It verifies if the gateway is valid only when submitting. I feel like validating on change would be better in terms of UX, but it can also be problematic as we have a 15 seconds timeout to account for the gateway to fetch the actual content.

License: MIT
Signed-off-by: Henrique Dias hacdias@gmail.com

License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias hacdias requested a review from lidel September 2, 2021 15:19
@hacdias hacdias self-assigned this Sep 2, 2021
@hacdias hacdias temporarily deployed to Deploy September 2, 2021 15:24 Inactive
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

I think checking on submit is perfectly fine for basic sanity check that protects users against URL that does not act as a gateway.

We could check on every change, but then we have to add "Testing..." status that disables submit button until test is done + account for various edge cases. Fine to do this as follow-up PR, but i'm merging this as-is because it's already an improvement ✨

@lidel lidel merged commit 5a1f72c into main Sep 2, 2021
@lidel lidel deleted the fix/1844 branch September 2, 2021 15:34
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.

Settings: validate public gateway before saving it
2 participants