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

Undefined or null showing in document root when primary domain isn't set #171

Closed
dshoreman opened this issue Oct 22, 2019 · 4 comments · Fixed by #172
Closed

Undefined or null showing in document root when primary domain isn't set #171

dshoreman opened this issue Oct 22, 2019 · 4 comments · Fixed by #172
Assignees
Labels
bug Something isn't working hacktoberfest Ideal issue for Hacktoberfest

Comments

@dshoreman
Copy link
Owner

If you create a new site and change the Project type before setting the primary domain, the document root will show as /var/www/undefined (see screenshot below).

The fix is probably as simple as setting a default value of '' for the primary domain in the Site vueX module — I don't believe it's currently set at all.

There is also a similar issue where, if you load an existing "new" site that hasn't had its primary domain configured, it will show null in place of undefined in the document root. Again this is probably just a case of setting a default empty value in the store.

image

@dshoreman dshoreman added bug Something isn't working hacktoberfest Ideal issue for Hacktoberfest labels Oct 22, 2019
@dshoreman dshoreman added this to To do in Projects, Apps and Redirects via automation Oct 22, 2019
@Mouzourides
Copy link
Contributor

Hey, I'd like to give this a go :)

@dshoreman
Copy link
Owner Author

All yours!

Mouzourides added a commit to Mouzourides/servidor that referenced this issue Oct 23, 2019
@Mouzourides
Copy link
Contributor

Hey there, I've played around with it a while and I've come to the conclusion that it is not as simple as setting the primary domain in the Site state. This is due to the fact that if this property is set to empty string, then when /api/sites is posted to, a 422 exception is thrown with the error message: The primary_domain is not a valid FQDN.

I think this makes sense, it should always be undefined or a valid domain name, not an empty string at anytime.

Due to this, I've instead implemented a solution that checks if primary domain is defined and is not, it replaces it with empty string when setting the document root attribute.

Only thing is it looks a little strange when project type is Laravel and document root is /var/www//public. Not really sure what to do instead in that situation though...

What do you think? :)

@dshoreman
Copy link
Owner Author

Thanks @Mouzourides, good thinking on the validation errors... That's probably the reason the optional fields were all left undefined in the first place.

We could update the last line to site.document_root = site.primary_domain ? var : ''; instead of the (... || '') where we set val initially, but to be honest I'm not too worried about it looking odd for Laravel projects. Just fixing the undefined/null output is good enough for now.

The whole form will probably need to be redesigned at some point anyway, especially when it supports non-web projects. Those won't even have a domain and the doc root will probably just be the home directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hacktoberfest Ideal issue for Hacktoberfest
Development

Successfully merging a pull request may close this issue.

2 participants