Trim domain to first word in server URL

[kanishk98]

If the app is unable to fetch the realm name, the server alias will now default to first word after the protocol scheme in the server URL.

---

What's this PR do?

Changes serverConf object so that if the app is unable to save the correct realm name, the sidebar receives the first character of the URL after ://.

Any background context you want to provide?

Discussion at #675

Screenshots?

You have tested this PR on:

• [x] Windows
• [ ] Linux/Ubuntu
• [ ] macOS

[akashnimare]

Would it make sense to put the trimDomain function in the new request-util.js ?

[vsvipul]

Would it make sense to put the trimDomain function in the new request-util.js ?

No, it wouldn't i guess. The request-util returns header options for all requests in domain-util, and does not return alias name.

[abhigyank]

There are three cases of url here :-

• abc.domain.com - Show a
• zulip.domain.com / server1.zulip.domain.com - Show d / show s
• domain.com - show d (very rare case)

It is easy for us to implement for the first case as done in this case. The problem arises when we want to distinguish between first and second case which I feel is impossible for us to do. I am not sure if showing z in case of 2nd case an improvement from the the current implementation of showing the defaultIcon or is infact an increment of poor UX. Really unsure as to what the way forward should be here in this case.

[kanishk98]

zulip.domain.com / server1.zulip.domain.com - Show d / show s

@abhigyank correct me if I'm wrong, but the problem here is just the word zulip in the server URL, right? If that's the case, we can check if the first word of the URL after the protocol scheme starts with zulip. If so, then we assume that the URL is of the form zulip.domain.com and show d. The current case should show s in case of server1.zulip.domain.com, and I'm guessing that's acceptable. (If not, we can simply check for zulip and find the next immediate word that exists. Of course, we'll have to make sure we don't overrun case 1 when doing this).

[kanishk98]

Please let me know if any further changes are required here.

[abhigyank]

but the problem here is just the word zulip in the server URL, right? If that's the case, we can check if the first word of the URL after the protocol scheme starts with zulip. If so, then we assume that the URL is of the form zulip.domain.com and show d.

I don't want us to hard-code here (I am never in favour of hard-coding anything! :sweat_smile: ) The reason being we don't want to assume that its always zulip in such a subdomain. I am not sure what is the right way to go here. :confused:

[kanishk98]

I agree, hard coding is a pretty hacky solution. I’m also unsure what we should do. :(

On Thu, 4 Apr 2019 at 10:44 PM, Abhigyan Khaund [email protected] wrote:

but the problem here is just the word zulip in the server URL, right? If that's the case, we can check if the first word of the URL after the protocol scheme starts with zulip. If so, then we assume that the URL is of the form zulip.domain.com and show d.

I don't want us to hard-code here (I am never in favour of hard-coding anything! 😅 ) The reason being we don't want to assume that its always zulip in such a subdomain. I am not sure what is the right way to go here. 😕

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zulip/zulip-electron/pull/693#issuecomment-479985567, or mute the thread https://github.com/notifications/unsubscribe-auth/AXehUYK57iQ_R2NzwXYh1xq82pn169xeks5vdjLugaJpZM4cUTWD .

[kanishk98]

@rishig would you mind having a look at this whenever you're free and telling us what you think?

[rishig]

I think we should just show some kind of error state if the realm doesn't have an avatar set (e.g. a box with a ?).

[kanishk98]

I think we should just show some kind of error state if the realm doesn't have an avatar set (e.g. a box with a ?).

I think that'll be a good solution when we don't have this kind of behaviour come up frequently (which is the case right now). When @akashnimare is able to find time to review #714 and we cut down on the number of times this fallback icon needs to be used, we can use the ?. Sound good?

[zulipbot]

Heads up @kanishk98, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.