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

No error when trying to add an instance without organisation #596

Closed
superbiche opened this issue Nov 18, 2018 · 11 comments · Fixed by #750
Closed

No error when trying to add an instance without organisation #596

superbiche opened this issue Nov 18, 2018 · 11 comments · Fixed by #750

Comments

@superbiche
Copy link

superbiche commented Nov 18, 2018

What's wrong
Zulip desktop client doesn't display any error or information when trying to add a Zulip instance without any Organisations in it.

Steps to reproduce

  • Start a fresh, properly configured Zulip instance. Do not create any organisation
  • Open the Electron app
  • Try to add this instance

Expected
Zulip should give a clear, helpful message so the user understands he needs to create the org first / ask the Zulip instance admin to create the org.

Possible resolutions
https://github.com/zulip/zulip-electron/blob/925fec71d5542dd124e0b7065023195dac364e12/app/renderer/js/utils/domain-util.js#L215 is looking for the realm icon and not expecting it to be missing.
Adding a else block here to render an alert would suffice.

OS
Mac OS Mojave 10.14.1

I can PR this if needed.

EDIT: I'm leaving https://zulip2.superbiche.co (my testing instance without org) as-is so you can easily reproduce this behavior. I'll shut it down when we've fixed it and checked it works.

@akashnimare
Copy link
Member

Yeah, it's a known issue. We'll fix this once we improve our validation logic, see https://github.com/zulip/zulip-electron/issues/573.

@superbiche
Copy link
Author

@akashnimare ok, I'm continuing the discussion in #573 for now. Thanks for the link

@superbiche
Copy link
Author

@akashnimare yeah it's related but I think we shouldn't wait to fix this particular issue in this particular situation, because it makes the self-hosted validation logic more robust while we think about a full refactor handling both zulipchat.com and self-hosted issues.

What do you think?

@timabbott
Copy link
Sponsor Member

I think a PR for this would be great; it seems like a small fix that is useful independent of when we end up doing #573.

@Nikhil-Vats
Copy link
Collaborator

Can I work on this @timabbott? Since I have already sent a PR for the previous issue I was working on.

@akashnimare
Copy link
Member

@NikhilPhalange feel free to work on this.

@Nikhil-Vats
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

Hello @NikhilPhalange, it looks like you've currently claimed 1 issue in this repository. We encourage new contributors to focus their efforts on at most 1 issue at a time, so please complete your work on your other claimed issues before trying to claim this issue again.

We look forward to your valuable contributions!

@Nikhil-Vats
Copy link
Collaborator

Nikhil-Vats commented Jan 28, 2019

I am not able to understand this issue as soon as I initiate a zulip instance, I am shown this which is okay, I guess. Please see the screenshot attached, this is the page which comes after we add an instance. @superbiche @akashnimare
screenshot 106

Till then I am working on some other issue, I hope it's fine.

@kanishk98
Copy link
Collaborator

@Nikhil-Vats I think the issue is that when you're setting up a Zulip server, then the install script prints a link that you then visit in your browser to set up the organization.
@superbiche is saying that the desktop app should alert the user when a server without an org is added to the app.
Let us know if you can look into this issue right now. If not, someone else can take it up.

@kanishk98
Copy link
Collaborator

@zulipbot claim

akashnimare pushed a commit that referenced this issue Jun 12, 2019
This PR removes .ogg file check (supported only by very old servers). Other enhancements in server validation logic -
* Reject domains with no organizations. 
* Convert validation methods to async await
* Add messages.js for returning error message strings.

Fixes: #596, #573.
kanishk98 added a commit to kanishk98/zulip-desktop that referenced this issue Jun 18, 2019
This PR removes .ogg file check (supported only by very old servers). Other enhancements in server validation logic -
* Reject domains with no organizations. 
* Convert validation methods to async await
* Add messages.js for returning error message strings.

Fixes: zulip#596, zulip#573.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants